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

Utilize a high_memory Option in FragPipe Tool #1261

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

reid-wagner
Copy link
Contributor

While more typical FragPipe jobs are successful on GalaxyEU with the current settings, some larger searches are running out of memory. This would allow us to set a high_memory option in FragPipe to increase allocated resources.

@reid-wagner reid-wagner changed the title Utilize a high_memory option in FragPipe tool to increase memory and cores Utilize a high_memory Option in FragPipe Tool Jul 18, 2024
@@ -1444,3 +1444,10 @@ tools:
toolshed.g2.bx.psu.edu/repos/galaxyp/fragpipe/fragpipe/.*:
cores: 8
mem: 56
rules:
- if: |
Copy link
Member

Choose a reason for hiding this comment

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

if think - id is missing, please look at the hifi example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm still getting AttributeError: 'Tool' object has no attribute 'params_from_strings' despite modeling hifiasm pretty closely. Any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bgruening - I'm trying to dig into this further but I think I have hifiasm pretty well replicated, and I'm wondering if there's any piece I'm missing here?

Copy link
Member

@sanjaysrikakulam sanjaysrikakulam Jul 24, 2024

Choose a reason for hiding this comment

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

When I run tpv dry-run on the toolshed.g2.bx.psu.edu/repos/bgruening/hifiasm/hifiasm/0.19.9+galaxy0, I get the same error. I don't know whether this rule was tested before or if something changed in the upstream codebase that handles the tool entity. I will have a further look and update you.

Copy link
Contributor

@mira-miracoli mira-miracoli Jul 24, 2024

Choose a reason for hiding this comment

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

I tried dry running with older versions, back to 2.1.0 and the error remained the same.
My Galaxy packages are all 22.1.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking into this @sanjaysrikakulam and @mira-miracoli!

That error is a known issue in the bioconda recipe and doesn't prevent the tool from running. We shouldn't need to set up the Java env either as the wrapper scripts do this already using the GALAXY_MEMORY_MB variable.

Copy link
Member

Choose a reason for hiding this comment

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

The jobs fail as soon as they hit those errors. Maybe I am missing some parameters (I left everything to defaults)? Please test the tool and let us know if it works and we can proceed with this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these the automated tests as defined in the <tests> element that are running? These ran successfully in the most recent update here (galaxyproteomics/tools-galaxyp#769), and we do have successful jobs on the EU site as well.

Copy link
Member

Choose a reason for hiding this comment

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

Nope. I started a job through the UI. For the data, I clicked on Generate Tour and did not change any parameters in the tool's UI. The jobs failed with the above error. If your jobs run in the EU without errors, we can proceed with this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. I'm not familiar with tours but we haven't built that for FragPipe.

Copy link
Member

@sanjaysrikakulam sanjaysrikakulam left a comment

Choose a reason for hiding this comment

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

Let's try.

@reid-wagner
Copy link
Contributor Author

Thank you!

@sanjaysrikakulam
Copy link
Member

sanjaysrikakulam commented Jul 24, 2024

We will deploy this tomorrow morning.

@bgruening bgruening merged commit 8b03fef into usegalaxy-eu:master Jul 24, 2024
3 of 4 checks passed
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