-
Notifications
You must be signed in to change notification settings - Fork 84
Add FAIRyMAGs Taxonomic Binning Evaluation Workflow #923
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
Add FAIRyMAGs Taxonomic Binning Evaluation Workflow #923
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @SantaMcCloud, this looks quite interesting! Here's a few things I noticed on a first pass through. Please also move all test data larger than 100KB to zenodo and delete the files from the test-data folder. Is binninge a typo in taxonomic-binninge-evaluation ? if so please also rename the folder.
@paulzierep @bebatut can you review the workflows ?
workflows/microbiome/taxonomic-binninge-evaluation/.dockstore.yml
Outdated
Show resolved
Hide resolved
workflows/microbiome/taxonomic-binninge-evaluation/.dockstore.yml
Outdated
Show resolved
Hide resolved
workflows/microbiome/taxonomic-binninge-evaluation/FAIRyMAGs-GTDB-Tk-subworkflow.ga
Outdated
Show resolved
Hide resolved
...ws/microbiome/taxonomic-binninge-evaluation/FAIRyMAGs-taxonomic-binning-evaluation-tests.yml
Outdated
Show resolved
Hide resolved
...ws/microbiome/taxonomic-binninge-evaluation/FAIRyMAGs-taxonomic-binning-evaluation-tests.yml
Outdated
Show resolved
Hide resolved
...ws/microbiome/taxonomic-binninge-evaluation/FAIRyMAGs-taxonomic-binning-evaluation-tests.yml
Outdated
Show resolved
Hide resolved
workflows/microbiome/taxonomic-binninge-evaluation/FAIRyMAGs-taxonomic-binning-evaluation.ga
Outdated
Show resolved
Hide resolved
workflows/microbiome/taxonomic-binninge-evaluation/GTDB2NCBI-TaxID-Subworkflow.ga
Outdated
Show resolved
Hide resolved
Test Results (powered by Planemo)Test Summary
Errored Tests
Passed Tests
|
|
@mvdbeek thank you for the review i will work on them later! I thought since all of the subworkflows are included in the main workflow they has to be submitted aswell. I can remove them if you want or keep it however you like. The second workflow can be used outside but i dont think this wf will be often used. I only have 1 question and 1 Problem: Q: When moveing the Problem: When testing this WF i run into an error which might be because of the Galaxy logic. The first subworkflow runs GTDB-Tk and this outputs are provided for the second subworkflow. Since Galaxy start to create the jobs for the second subworfklow without wating till the inputs are done from GTDB-Tk the status of this run is marked as failed which will stop the test and let them fail. I did try it out multiple times and always at this point its break and report me them as failed while on The second subworkflow is the only one passing the test so this should be the problem. |
Please don't do that, you should write assertions against the data, e.g. for things you expect to be present in the outputs.
So that would be an extremely sever bug that, while not impossible, is kind of unlikely to happen. Can you use that database or do we need to update what is in CVMFS ? |
Since the outputs are very small which are comapred (between 3 - 45 KB) i think this sould not be a problem since the delta is also very low (1500 bytes). I did remove all files over 100KB to Zenodo ( e21be0b) If you still dont want this for the test, the files size comparing, i will change it. I just didnt change it yet since if you dont want to have the subWFs uploaded here all of this kind of test will be deleted.
Okay this is the only thing i could see other thing i couldnt explain but if these error are happening now you might can help me to find the error then to fix it since manually testing doesnt help me to find it.
Yes this DB can used. The problem was the timestamp when the DB was created. I did change it (81c9fd6) now so it should not run into an error this time i hope. Thank you for the help! |
Test Results (powered by Planemo)Test Summary
Errored Tests
Passed Tests
|
|
@mvdbeek could you check now why the test errored? I didnt found anything useful |
Test Results (powered by Planemo)Test Summary
Errored Tests
|
that was likely bug in planemo, I've released a new version and re-triggered the tests 🤞! |
Test Results (powered by Planemo)Test Summary
Errored Tests
|
Test Results (powered by Planemo)Test Summary
Errored Tests
|
still failing and i dont know where the log is now. Seems still be a bug since i runs normal on galaxy |
|
Ah thank you thougt so there is a bug somewhere. The test is runnning and it seems that it works now. Thank you for the fix! |
|
| elements: | ||
| - class: File | ||
| identifier: forward | ||
| location: https://zenodo.org/records/16740696/files/Reads_forward.fastqsanger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is 3GB, that is far too large. I would suggest making this a megabyte or less.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was the first test data which did run. I will minimize it but it will take time since only 1 tool need specific reads it seems to work. I will pick some and try it with it to see if it works better and faster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mvdbeek i did change the file size now and i hope the current size is fine for it!
The i did test the WFs locally on the .EU serve via planemo and all test where fine i hope that there will no problem in github this time and if so could you help me to reolve them? Thank you in advance for this!!
Test Results (powered by Planemo)Test Summary
Errored Tests
|
|
The 143 status code indicates the github runner ran out of resources. If you have an invocation on usegalaxy.org you could look at what step consumes most memory or disk |
It didnt run on .org yet since some tools are not installed. I will start a run in the evinign to have the insider and based on this i will maybe try to cut some contigs from the input to have lower numbers |
|
This PR will be on hold since some adjustment has to be done since GTBD-Tk use to much memorey which make the test errors out always |
FOR CONTRIBUTOR:
FOR REVIEWERS:
This workflow does/runs/performs … xyz … to generate/analyze/etc …namefield should be human readable (spaces are fine, no underscore, dash only where spelling dictates it), no abbreviation unless generally understood-) over underscore (_), prefer all lowercase. Folder becomes repository in iwc-workflows organization and is included in TRS id