Make init function atomic

[BenjaminBossan]

I just had a look at init and it's not atomic:

skops/skops/hub_utils/_hf_hub.py

Lines 92 to 122 in 41716e5

	def init(*, model: Union[str, Path], requirements: List[str], dst: Union[str, Path]):
	"""Initialize a scikit-learn based HuggingFace repo.
	Given a model pickle and a set of required packages, this function
	initializes a folder to be a valid HuggingFace scikit-learn based repo.
	Parameters
	----------
	model: str, or Path
	The path to a model pickle file.
	requirements: list of str
	A list of required packages. The versions are then extracted from the
	current environment.
	dst: str, or Path
	The path to a non-existing or empty folder which is to be initialized.
	Returns
	-------
	None
	"""
	dst = Path(dst)
	if dst.exists() and next(dst.iterdir(), None):
	raise OSError("None-empty dst path already exists!")
	dst.mkdir(parents=True, exist_ok=True)
	shutil.copy2(src=model, dst=dst)
	model_name = Path(model).name
	_create_config(model_path=model_name, requirements=requirements, dst=dst)

E.g. if the last step (_create_config) fails, the user fixes that step, then calls init again, it will suddenly fail in an earlier step because the directory is not empty anymore. A possible fix would be to create a temp directory, put everything there, and in the last step move that directory to the destination. @adrinjalali @merveenoyan WDYT?

[adrinjalali]

I'm happy with your idea, except that I'd rather not copy the model file into the temp folder, then to the destination folder, those files might be quite large.

[BenjaminBossan]

But the last step is just a move operation, so no additional copy is being made. Or do I misunderstand?

[adrinjalali]

I'd try with a model file which is like 10GB large and see if what you're suggesting adds some odd overhead, if not, I'm happy.

[BenjaminBossan]

$ du -h ubuntu-22.04-desktop-amd64.iso 
3,5G	ubuntu-22.04-desktop-amd64.iso
$ time mv ubuntu-22.04-desktop-amd64.iso{,-cp}

real	0m0,002s
user	0m0,000s
sys	0m0,002s

Is that enough?

[adrinjalali]

This operation you have doesn't actually move the data. a TempFolder would be probably on a tempfs and would need to be actually copied to an ext4, btrfs, etc file system. (I'm not sure how temps work in OSs other than linux though).

[BenjaminBossan]

Okay, I'll check with an example that's closer to what it would actually look like and will post an update.

[adrinjalali]

Also, we could instead make sure the copy is done at the end, and only do other operations on the temp folder, and keep the copy only once.

[BenjaminBossan]

I ran this quick and dirty test:

import os
import shutil
import tempfile
import time
from pathlib import Path


def main():
    with tempfile.TemporaryDirectory(prefix="skops-test-foo") as path:
        path = Path(path)
        file = Path(os.path.expanduser('~')) / "Downloads" / "ubuntu-22.04-desktop-amd64.iso"

        tic = time.monotonic()
        shutil.copy2(file, path)
        toc = time.monotonic()
        print(f"Copying took {toc - tic} sec")

        with tempfile.TemporaryDirectory(prefix="skops-test-bar") as path2:
            tic = time.monotonic()
            shutil.move(path / "ubuntu-22.04-desktop-amd64.iso", path2)
            toc = time.monotonic()
            print(f"Moving took {toc - tic} sec")


if __name__ == '__main__':
    main()

My results:

Copying took 1.769518501998391 sec
Moving took 9.446701733395457e-05 sec

EDIT: Just to note, first copy took 15 sec, maybe disk cache? But the move operation was always extremely fast.

But then again, I don't use tempfs (don't think it's used by default), so YMMV.

we could instead make sure the copy is done at the end, and only do other operations on the temp folder, and keep the copy only once.

Not quite sure what you mean. My main goal would be to make these kinds of i/o tasks idempotent, I think that can provide a lot of value to users.

[adrinjalali]

Here's 10G on my system:

/tmp ⌚ 12:46:03
$ time head -c 10G /dev/urandom > sample.txt
head -c 10G /dev/urandom > sample.txt  0.99s user 26.08s system 99% cpu 27.100 total

/tmp ⌚ 12:46:40
$ time mv sample.txt ~
mv sample.txt ~  0.09s user 9.05s system 44% cpu 20.593 total

copy and move take a similar amount of time.

[BenjaminBossan]

I wouldn't exactly say the time is similar, I guess it depends ;)

Btw. on my machine:

/tmp$ time head -c 10G /dev/urandom > sample.txt

real	2m5,814s
user	0m1,524s
sys	2m3,301s

/tmp$ time mv sample.txt ~

real	0m38,714s
user	0m0,126s
sys	0m9,674s

What kind of disk do you have :D

Anyway, if you have another suggestion to achieve the goal, I'm up for it.