search

flatironinstitute / disBatch

Tool to distribute a list of computational tasks over a pool of compute resources. The pool can grow or shrink.
Apache License 2.0
39 stars 8 forks source link

kvsstcp pip dependency #21

Closed blackwer closed 2 years ago

blackwer commented 3 years ago

This branch bumps the submodule kvsstcp to version 1.2 and then also points setup.py to the same commit. Currently untested with the database functionality, so that needs to be verified before merging.

blackwer commented 3 years ago 
pip install /path/to/disbatch
sbatch -N1 --wrap 'disBatch.py -g taskfile'
sbatch -N1 --wrap '/path/to/disbatch/disBatch.py -g taskfile'

Now both work as expected

blackwer commented 3 years ago

And now

pip install /path/to/disbatch
sbatch disBatch taskfile
sbatch /path/to/disbatch/disBatch taskfile

both work as expected

njcarriero commented 3 years ago

Thanks for these modifications. It took me a little while to fully appreciate the approach.

Somewhat a question of tastes, but I would prefer DISBATCH_ROOT to have the "bin" trimmed before setting, so it is actually set to the root of the install. Then perhaps it would make sense to introduce a second env variable DISBATCH_BIN?

Was the "echo" (line 5) in dbUtils.sh there for debugging? Is it still needed?

Is the a naming convention that motivates calling the python site-package "disbatch" as opposed to "disBatch"?

blackwer commented 3 years ago

Somewhat a question of tastes, but I would prefer DISBATCH_ROOT to have the "bin" trimmed before setting, so it is actually set to the root of the install. Then perhaps it would make sense to introduce a second env variable DISBATCH_BIN?

I agree. It's a bit confusing and I didn't like it either. I'll see if I can avoid a second environment variable and keep DISBATCH_ROOT to be properly consistent without making things uglier than they need to be.

Was the "echo" (line 5) in dbUtils.sh there for debugging? Is it still needed?

This was removed in the last commit

Is the a naming convention that motivates calling the python site-package "disbatch" as opposed to "disBatch"?

It's a PEP styling thing. See: https://www.python.org/dev/peps/pep-0008/#package-and-module-names

Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.

We're of course free to do whatever we want, but it seemed like a minor thing.

blackwer commented 2 years ago

Somewhat a question of tastes, but I would prefer DISBATCH_ROOT to have the "bin" trimmed before setting, so it is actually set to the root of the install. Then perhaps it would make sense to introduce a second env variable DISBATCH_BIN?

Sorry I put this off so long. I thought this was going to be more complicated. This is fixed in https://github.com/flatironinstitute/disBatch/pull/21/commits/76d5ebffd1a0db67a2817db61b0013659b7d2c34