sct-pipeline / csa-atrophy

Evaluate the sensitivity of atrophy detection with SCT
https://csa-atrophy.readthedocs.io/
MIT License
1 stars 0 forks source link

configuration yaml file #25

Closed PaulBautin closed 4 years ago

PaulBautin commented 4 years ago

Many arbitrary parameters are dispersed in the code. Creating a config file regrouping these parameters will simplify the code use. First idea was to parse the yaml file directly in the process data bash script, however this was not as easy as using a python parser script. Moreover the use of a python yaml parser will allow generalization to other scripts.

DONE:

FIX #22 Not quite ready for review

PaulBautin commented 4 years ago

Up to now, user must enter filenames in '"/manual_labeling_correction.sh" script to manually correct errors of labeling. This can be avoided if user enters filenames in the config.yaml file. Moreover, I have decided to remove parallel processing on R_COEFS (a process_data.sh variable) as transformation values were not properly stored in transfo_values.csv file.

DONE:

FIX #23

PaulBautin commented 4 years ago

'"/manual_labeling_correction.sh" is outdated by new python script present on spine generic https://github.com/spine-generic/spine-generic/pull/103, @jcohenadad should i upgrade for new process? Other than that current process has been tested and would be ready for review.

jcohenadad commented 4 years ago

@jcohenadad should i upgrade for new process?

can you open an issue to address that in a subsequent PR, once the python script in spine-generic is completed

jcohenadad commented 4 years ago

https://github.com/sct-pipeline/csa-atrophy/pull/25#issuecomment-656392962 states:

Other than that current process has been tested and would be ready for review.

https://github.com/sct-pipeline/csa-atrophy/pull/25#issue-446468497 states:

Not quite ready for review

conclusion: NEVER write stuff in the PR body that could lead to such confusion--

solution: use the "draft" mode if PR is not ready for review. When ready, remove "draft mode" and assign someone.

PaulBautin commented 4 years ago

Good to know, i found how to convert PR back to draft mode. https://github.blog/changelog/2020-04-08-convert-pull-request-to-draft/

PaulBautin commented 4 years ago

When running sct_run_batch -config config.yaml, many unknown keys are displayed due to other variables in yml config file. Is it better to separate config files by script? Should there be a yml file especially for sct_run_batch? @jcohenadad

ERROR:

configuring /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "transfo" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "PATH_SEGMANUAL" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "rescaling" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "stats" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "contrast" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "PATH_ORIGINAL_CSV" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "FILES_SEGMANUAL" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "PATH_DESTINATION_RESULTS" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "PATH_ORIGINAL_RESULTS" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k))) /home/paul/github/spinalcordtoolbox/scripts/sct_run_batch.py:250: UserWarning: Unknown key "n_transfo" found in your configuration file, ignoring. 'Unknown key "{}" found in your configuration file, ignoring.'.format(k)))

jcohenadad commented 4 years ago

@PaulBautin have you tried using -script-args if you want to pass custom env variables to your script?

also, if you have errors with sct_run_batch, i suggest you open an issue in SCT, describing the observed behaviour, syntax, version, input yml, etc. so the team can reproduce

PaulBautin commented 4 years ago

@jcohenadad, I think this branch can be merged. I would like to do a csa-atrophy project release before processing a very large dataset.

jcohenadad commented 4 years ago

https://github.com/sct-pipeline/csa-atrophy/pull/25#issuecomment-667549055 is unanswered. All comments should be answered otherwise the person doing the review thinks the comment was missed.

PaulBautin commented 4 years ago

Sorry about that @jcohenadad , i have opened an issue in SCT https://github.com/neuropoly/spinalcordtoolbox/issues/2825 to answer comment https://github.com/sct-pipeline/csa-atrophy/pull/25#issuecomment-667255378. I think there is no need for custom env variables (-script-args) in this case. Or does it? Pipeline works well (except for the unnecessary comments mentioned above).