support ripping with docker in parallel #8
Conversation
|
Can run in parallel with e.g. (for docker image name pv): |
| --shm-size=1g \ | ||
| --env=TZ=America/Los_Angeles \ | ||
| ${1} | ||
| ## Support running in parallel: |
There was a problem hiding this comment.
I'm curious why you choose to do this in a shell script, compared to launching multiple jobs (or an array job) via slurm?
The reason I ask is because this seems to be adding some complexity around xvfb and locking (which looks tricky and is just one more thing that could break....), whereas parallel launching via slurm is straightforward. Also, it gets tricky to manage ending gracefully and/or doing retries in case where one or more of the parallel threads fails for some reason.
There was a problem hiding this comment.
I've been running this script on my local computer to avoid filesystem slowness, so this is nice for parallelization, but perhaps better to be in new script..?
| @@ -246,8 +245,9 @@ def run_suite2p(hdf5_list, dirname_output, mdata): | |||
| fs_param = 1. / (mdata['period'] * z_planes) | |||
|
|
|||
| # Load suite2p only right before use, as it has a long load time. | |||
| import suite2p | |||
There was a problem hiding this comment.
This loads in all of suite2p (which includes the gui stuff and the detection modules). It's not fatal, but I've found suite2p to be heavy weight. So I had just imported the subset of libraries (run_s2p) that we use below.
There was a problem hiding this comment.
latest version of suite2p refactored a bit, e.g. need suite2p.default_ops() but maybe not fully necessary
two-photon/rip.py
Outdated
| @@ -15,7 +15,7 @@ | |||
| # Ripping process does not end cleanly, so the filesystem is polled to detect the | |||
| # processing finishing. The following variables relate to the timing of that polling | |||
| # process. | |||
| RIP_TOTAL_WAIT_SECS = 3600 # Total time to wait for ripping before killing it. | |||
| RIP_TOTAL_WAIT_SECS = 10 # Total time to wait for ripping before killing it. | |||
There was a problem hiding this comment.
Doesn't this kill the ripper after one pass through the while loop at line 116? That seems too soon, no?
There was a problem hiding this comment.
oops, I had changed this to be a really large number as a hack to fix voltagerecording output and meant to revert back to previous
| @@ -35,6 +35,7 @@ def determine_ripper(data_dir, ripper_dir): | |||
| version = root.attrib['version'] | |||
|
|
|||
| # Prairie View versions are given in the form A.B.C.D. | |||
| # TODO: allow match when minor version mismatches, eg fall back to `version = '5.5.1.1'` | |||
There was a problem hiding this comment.
Currently, this uses the major+minor to determine which ripper version to use. It seems, at least anecdotally, that the minor version is important.
What behavior are you expecting on a minor version mismatch?
One improvement I see is that we should throw an error right here if the ripper version is not in the repo.
There was a problem hiding this comment.
yes you're right i think minor version is sometimes (usually) important...annoying...
There was a problem hiding this comment.
Then this TODO probably isn't necessary. We currently handle 5.4 vs 5.5, and I'd like the code to fail loudly (not silently try something we haven't tested) if it detects anything else.
Partial resolution to #7