tools-iuc
tools-iuc copied to clipboard
maaslin2 version update to 1.16.0
FOR CONTRIBUTOR: [* ] I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
- [ ] License permits unrestricted use (educational + commercial)
- [ ] This PR adds a new tool or tool collection [* ] This PR updates an existing tool or tool collection
- [ ] This PR does something else (explain below)
I am getting the following error while passing multiple reference factors.
Also any idea why residuals output is empty ?
I have removed the test case 4 and 5 with fixed_effect=NULL. Please let me know if I am missing something else.
Hi @paulzierep , @bernt-matthias I have made some changes in the test cases , as setting fixed_effect =NULL was taking infinite time to run, so I changed it. Can you please go through this and let me know if its ready to go now?
Hi @bernt-matthias Thanks for raising doubts. Yes the request was answered and I have depicted the use of multiple characteristic variable in 4th test case. Turned out, comma was to be used instead of semi-colon. Also in the previous version, we added last two test cases where the value of fixed_effects and Random_effects was set NULL in test case 4 and just fixed_effects was NULL in 5th test case. In this version, setting fixed_effects as NULL does not work somehow. The test command takes infinite time to run. So I removed those test cases and added a new test case showing use of reference.
About the residuals being empty, I will raise an issue again. https://forum.biobakery.org/t/residual-rds-output-file-empty-in-maaslin2-version-1-16-0/7129
Hi @paulzierep , @bernt-matthias
I have updated the test cases accordingly but I am unable to understand why 2 checks are failing . Can you please guide me on this?
Independent of this: can you switch to the bioconductor-maaslin2 requirement: https://github.com/bioconda/bioconda-recipes/pull/48229
I have updated the test cases accordingly but I am unable to understand why 2 checks are failing
Me neither. R errors messages .. as usual completely unusable.
It seems that this is the error we get for all tests:
Error in checkForRemoteErrors(val) :
4 nodes produced errors; first error: object 'current_args' not found
Calls: Maaslin2 ... clusterApply -> staticClusterApply -> checkForRemoteErrors
Execution halted
Ones again this is something we should check in the biobakery forum.
You said that only 2 checks are failing, could you show me an example where the test works? Does the tool work locally ? This might actually be a bioconda issue ? Can you try with bioconductor-maaslin2 as suggested ?
@renu-pal Can you change \${GALAXY_SLOTS:-4} to 1 and push before our meeting please ?
This is just a gut feeling, but the error comes after:
2024-06-19 22:17:09.828373 INFO::Filtered feature names from variance filtering:
2024-06-19 22:17:09.829121 INFO::Running selected normalization method: NONE
2024-06-19 22:17:09.829815 INFO::Bypass z-score application to metadata
2024-06-19 22:17:09.830501 INFO::Running selected transform method: AST
2024-06-19 22:17:09.845002 INFO::Running selected analysis method: LM
2024-06-19 22:17:09.991965 INFO::Creating cluster of 4 R processes
And when running the tool locally the part Creating cluster of 4 R processes is not shown ...
I did a rather stupid test locally, but using --cores 1 works and using --cores 4 fails with the given error. It seems that there is something internally with the R code that cannot work with multithreading.
Interestingly, the GitHub CI seems to assign 4 to \${GALAXY_SLOTS:-4} and locally planemo gives 1 to \${GALAXY_SLOTS:-4}; not sure how this can be influenced locally. But I think for now we could quickly fix the tool by using only 1 thread. And check with the biobakery folks if they have a fix for this.
Lets see: https://forum.biobakery.org/t/maaslin2-1-16-0-fails-with-more-then-1-cpu/7172
not sure how this can be influenced locally
You can just call GALAXY_SLOTS=4 planemo test ...
Hi @paulzierep , @bernt-matthias Finally all the checks have passed! Can this be merged now so I can start working on the tutorial :)
@bernt-matthias @bgruening wdyt good to merge ? The limit to 1 cpu is not ideal, but we can update when the devs fix it ...
Feel free to add the suggested change if you like. Then I can merge tomorrow.
Note that if there are Galaxy servers that enabled more resources (TPV) they will now waste them.
You mean if they assign more CPUs to maaslin2 via TPV ? we could add an entry here: https://github.com/galaxyproject/tpv-shared-database/blob/main/tools.yml I guess, but should not forget to remove it, ones it's fixed ...