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

nf-core launch --id xxx doesn't strip default params #976

Closed
ewels opened this issue Mar 26, 2021 · 4 comments
Closed

nf-core launch --id xxx doesn't strip default params #976

ewels opened this issue Mar 26, 2021 · 4 comments
Labels
bug Something isn't working high-priority

Comments

@ewels
Copy link
Member

ewels commented Mar 26, 2021

If you use the web launch tool directly, after filling in the form you are given a nf-core launch command you can run, eg, nf-core launch --id 1616773347_cbd91cd92761. Running this then pulls the input params JSON. It should strip out all of the default parameters (the web preview shows this, and the command line launch does this), but it's not - it saves all parameters to the JSON file.

Need to add in the relevant strip parameters function into the relevant bit of code on the cli tool here.

@ewels
Copy link
Member Author

ewels commented Mar 26, 2021

Update: same effect when doing nf-core launch <pipeline> from the cli with the web GUI.

Using the command line wizard, the defaults are stripped.

@ewels
Copy link
Member Author

ewels commented Mar 26, 2021

So after investigating this a little more, it's not that it wasn't removing the defaults - it's that it wasn't removing stuff that didn't have a default. So it was loads of flags and things that were initialised as False.

Also then found some issues with the fact that the default values weren't being sanitised after coming back from the web, so could have booleans with default values of "true" instead of True.

@ewels ewels closed this as completed Mar 26, 2021
@ewels ewels reopened this Mar 26, 2021
ewels added a commit to ewels/nf-core-tools that referenced this issue Mar 26, 2021
* If a param doesn't have a default in the schema and the input is False / None / , strip it
* Sanitise the default values in the schema so that they are the correct type
* Don't surround command-line params with quotes if numeric

Fixes nf-core#976
@ewels ewels closed this as completed Mar 30, 2021
@nylander
Copy link

As a note, I see that when I run using the nfcore launch --id XXX syntax, I get a json file downloaded and saved in my cwd. If I compare this one it is more exhaustive than the minimal version I can see on the web page. Just as you described.

From the point of reproducibility, as a user I would really like to see all "relevant" options written in the file that is downloaded - even the ones with defaults. Then it would more easy, at least to me, to see what I actually did run.

The key word would be "relevant". You may, for example, have a lot of false settings default that doesn't have any effect on the analysis. However, as soon as you, e.g., say --run_bam_filtering, and change one setting, say --bam_mapping_quality_threshold 2, you immediately also have other parameters that are send to the relevant tools for this step of the workflow, right? I would like to see those settings in my json file, even if they had some defaults.

/Johan

@ewels
Copy link
Member Author

ewels commented Apr 3, 2021

@nylander - I like the idea, but this would be extremely difficult to implement. The pipeline author would have to manually curate all related parameters which would be a lot of work for not much gain (in my opinion).

We don't save all options by default as this makes it more difficult to use the parameters file again in the future with a different version of the pipeline (for example, defaults may change for a good reason). However, nf-core launch does have a flag to instruct it to save everything to the JSON file if you wish.

Also note that the pipeline runs themselves log every parameter used, so you can refer back to that. We have also discussed / loosely planned to save a JSON file that could be used to rerun the pipeline as one of the pipeline outputs (though this hasn't been written yet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority
Projects
None yet
Development

No branches or pull requests

2 participants