PennLINC / babs

BIDS App Bootstrap (BABS)
https://pennlinc-babs.readthedocs.io
MIT License
5 stars 5 forks source link

[ENH] refactoring of babs_submit function and updator of job status table #99

Closed djarecka closed 1 year ago

djarecka commented 1 year ago

@zhao-cy - I've started also working on the option of adding job arrays (it's important for our group), and I've created a new function to remove some duplication in babs_submit, especially because with the job array option I would add even more duplication. I've decided to create a separate PR for this, so this doesn't mix with the array changes

zhao-cy commented 1 year ago

Oh this is a great idea! Thank you @djarecka !

djarecka commented 1 year ago

@zhao-cy - I've also created a function prepare_job_ind_list that prepare list of job indices that should be submitted. I think this simplify babs_submit, but let me know what you think.

Today, I only tested this for single session

zhao-cy commented 1 year ago

@djarecka I think adding prepare_job_ind_list() function is smart! I added two minor comments on the code changes - see above. Let me know if you cannot see my comments! Thank you Dorota!!

djarecka commented 1 year ago

I've also run a couple test for for multi-session!

zhao-cy commented 1 year ago

@djarecka Wonderful! I believe you have tested out different babs-submit arguments (like --job, --count) and babs-status arguments (like --resubmit) under different scenarios (e.g., some jobs are pending/running/failed)? I'm also happy to help testing out if needed! Just let me know!

And once these testings are done let me know if you think this PR is ready go? If so I'll review your code and if everything looks good to me I'll merge it to main.

djarecka commented 1 year ago

I've tried to test for various scenarios for babs-submit (with --job and --count) and babs-status, but I haven't tested --resubmit...

Perhaps it would be great if you can also run couple tests, just in case?

Otherwise, this is ready, for array I prefer to open a separate pr

zhao-cy commented 1 year ago

Sure sounds good! Will run some tests, review your code and merge to main!

zhao-cy commented 1 year ago

List of things I've tested using a multi-session dataset on MSI Slurm cluster: