nimh-dsst / dsst-defacing-pipeline

Defacing algorithm to improve and evaluate accuracy for large datasets.
4 stars 2 forks source link

BIDS App preparation and serial/parallel behavior #21

Closed ericearl closed 1 year ago

ericearl commented 1 year ago

I started to move away from the "BIDS-App-like" behavior and started moving toward "BIDS-App" behavior like this:

https://github.com/bids-apps/example/blob/master/run.py

This involved changing the CLI. I also updated the serial and parallel behavior to rely on a --n-cpus or -n argument provided an integer. I also recognize the README still needs an overhaul here, but the functionality is solid and tested in all four cases of:

  1. Local serial run on subjects without sessions
  2. Local serial run on subjects with sessions
  3. Local parallel run on subjects without sessions
  4. Local parallel run on subjects with sessions

All "Local" runs were performed on curium. I'm happy to talk about this at your convenience.

Full disclosure, I have not tested the latest changes to the VisualQC prep'ed files. But I can quickly now if you'd prefer.

Arshitha commented 1 year ago

Thanks for the PR @ericearl ! I'll test out the branch sometime mid-next week and add comments here. I'm working on a few issues on integrate-into-bids-tree branch, and I don't want to make new changes until I've resolved functional issues pertaining to running the pipeline on HPC and visualizing on TurboVNC.

ericearl commented 1 year ago

@Arshitha You're welcome! If you want to point me to data that I didn't curate I can continue testing and iterate through usage and non-usage of flags, etc. I've been testing on curium with the C11 data and it's really easy to launch concurrently (I'm re-running tests since my last tests were before the merge). Let me know if you want another set of eyes or hands on the HPC and viz issues on TurboVNC.

Arshitha commented 1 year ago

@ericearl Thank you for you patience while I tested and reviewed this! PR has been to integrate-into-bids-tree branch but I'll merge it to main now.