Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass on gpu options #193

Merged

Conversation

MichielCottaar
Copy link
Contributor

Passes on the --no-gpu and --cuda-version flag from DiffPreprocPipeline.sh to DiffPreprocPipeline_Eddy.sh.

Also contains two other minor fixes:

  • replaces some tabs with spaces, so that they are not mixed together
  • only print no_gpu when running DiffPreprocPipeline_Eddy.sh if no_gpu has actually been set to something

@MichielCottaar
Copy link
Contributor Author

This resolves #189

Copy link
Contributor

@mharms mharms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thx @MichielCottaar

@@ -379,7 +379,9 @@ get_options() {
echo " resamp_value: ${resamp_value}"
echo " ol_nstd_value: ${ol_nstd_value}"
echo " extra_eddy_args: ${extra_eddy_args}"
echo " no_gpu: ${no_gpu}"
if [ ! -z ${no_gpu} ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our convention in other scripts is to print argument values even if they are empty or used the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've adjusted this to print all argument values. To make the printed output more meaningful I set the default value from no_gpu (and SelectBestB0) to false rather than an empty string.

@@ -223,7 +223,7 @@ get_options() {
resamp_value=""
unset ol_nstd_value
extra_eddy_args=""
no_gpu=""
no_gpu="False"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our changing convention on the case across these scripts is annoying (False vs false vs FALSE), but I see that what you did in DiffPreprocPipeline_Eddy.sh is consistent with the rest of the script.

@glasserm
Copy link
Contributor

Is this ready now @coalsont ?

@glasserm glasserm merged commit cdc0733 into Washington-University:master Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants