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

metamorpheus initial commit #587

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jj-umn
Copy link
Member

@jj-umn jj-umn commented May 4, 2021

Add galaxy tools for the MetaMorpheus application.
MetaMorpheus is a bottom-up proteomics database search software with integrated post-translational modification (PTM) discovery capability.

Add galaxy tools for the MetaMorpheus application.
MetaMorpheus is a bottom-up proteomics database search software with integrated post-translational modification (PTM) discovery capability.
@jj-umn jj-umn added the wip label May 4, 2021
@jj-umn
Copy link
Member Author

jj-umn commented May 4, 2021

@bernt-matthias Here's my initial commit for MetaMorpheus.

It's still a little rough and needed plenty of testing. If you are willing to help with that, making sure that it meets the need of your clients, it would be much appreciated.

I haven't implemented tools for the GlycoSearchTask or the XLSearchTask as yet. I also want to check out using MetaMorpheus for Neoantigen HLA-bound searching: https://github.com/smith-chem-wisc/MetaMorpheus/wiki/1_New-Task:-Non-specific-cleavage-(%22No-Enzyme%22,-%22Non-specific%22)-searches

Copy link
Collaborator

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

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

Hey @jj-umn. This looks great.

If you like I could ask my colleague who was interested to test this.

@@ -0,0 +1,65 @@
<tool id="metamorpheus_calibration" name="MetaMorpeus Calibration" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" python_template_version="3.5">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<tool id="metamorpheus_calibration" name="MetaMorpeus Calibration" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" python_template_version="3.5">
<tool id="metamorpheus_calibration" name="MetaMorpeus Calibration" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="20.01">

]]></token>

<xml name="dissociation_options">
<option value="Any">Any</option>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you fix indentation here?

[CommonParameters]
TaskDescriptor = "CalibrateTask"
MaxThreadsToUsePerFile = 11
#if $common.set_common == 'yes'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to use sections instead of conditionals. Then you may skip this if-else and just use the defaults in the tool form (I guess they are the same anyway).

The disadvantage of conditionals is that they can not be changed in workflows.

Also we would know if there are no typos in the code that is currently in the if branch and the sytnax is correct (ie accepted by MM).

Copy link

Choose a reason for hiding this comment

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

These config yaml files may change by the version. They can be generated with metamorpheus -g, so if possible, it might be worth adding a test to make sure the default parameters and parameter names haven't changed since the last version.

MS3ChildScanDissociationType = "Unknown"
#end if
</token>
<xml name="mods_params">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed?

</requirements>
</xml>

<token name="@DEFAULT_COMMON_TOML@">
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the idea of this toml file? There seems to be some intersections with the others? Should the parameters be configurable?

#set $m = $re.match('^(\[.+\])$', $entry)
#if $m:
#set $category = $m.groups()[0]
#if $category not in $config['categories']:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think #if $category not in $config should work, i.e. it should not be necessary to store the categories in $config['categories'].

]]></token>

<token name="@MERGE_TOMLS@"><![CDATA[
#def merge_tomls($tomls, $merged):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I only checked the code roughly. Can you tell we how does this differs from just concatenating the files.

<expand macro="calibrated_outputs"/>
</outputs>
<tests>
<test>
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add expect_num_outputs .. which is always a good idea if there are outputs with filters.

Copy link
Collaborator

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

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

Tests are not running because .shed.yml is missing.

@jj-umn
Copy link
Member Author

jj-umn commented May 4, 2021

@bernt-matthias I suspect the tests will now fail because the toml datatype has only made it to the dev branch.

@bernt-matthias
Copy link
Collaborator

I suspect the tests will now fail because the toml datatype has only made it to the dev branch.

True. I think the 21.05 branch will become available today. Then we can adapt the pr workflow.

Then we should also use profile 21.05.

Add profile 21.05 for toml datatype
Fix metamorpheus_calibration.xml test output element name
@bernt-matthias
Copy link
Collaborator

@jj-umn just added 21.05 support for this repo: #589 .. if you merge/rebase with master tests should run.

@bernt-matthias
Copy link
Collaborator

Seems that MM tries to access a missing file / write to the installation dir.

Unhandled exception. System.UnauthorizedAccessException: Access to the path '/usr/local/lib/dotnet/tools/metamorpheus/CustomAminoAcids' is denied.
 ---> System.IO.IOException: Permission denied
   --- End of inner exception stack trace ---
   at System.IO.FileSystem.CreateDirectory(String fullPath)
   at System.IO.Directory.CreateDirectory(String path)
   at EngineLayer.GlobalVariables.WriteAminoAcidsFile()
   at EngineLayer.GlobalVariables.LoadCustomAminoAcids()
   at EngineLayer.GlobalVariables.SetUpGlobalVariables()
   at MetaMorpheusCommandLine.Program.Run(CommandLineSettings settings)
   at MetaMorpheusCommandLine.Program.<>c__DisplayClass3_0.<Main>b__1(CommandLineSettings options)
   at CommandLine.ParserResultExtensions.WithParsed[T](ParserResult`1 result, Action`1 action)
   at MetaMorpheusCommandLine.Program.Main(String[] args)
/usr/local/bin/metamorpheus: line 3:    12 Aborted                 (core dumped) dotnet $DOTNET_TOOLS/metamorpheus/CMD.dll $@

@jj-umn
Copy link
Member Author

jj-umn commented May 6, 2021

@bernt-matthias @bgruening @acesnik
I think MetaMorpheus has option:
--mmsettings [Optional] Path to MetaMorpheus settings
To solve that issue. I'll check into adding it to the galaxy tools.

Add --mmsettings option to  prevent attempts to write to  install code directory
@bernt-matthias
Copy link
Collaborator

We will need a fatal_oom stdio regexp for Run failed, Exception: Exception of type 'System.OutOfMemoryException' was thrown. (e.g. just OutOfMemoryException). .. appears on stdout

Don't expect exact number of reported datapoints
MetaMorpheus param fixes - need double quotes around string types
@bernt-matthias
Copy link
Collaborator

Good hints here: smith-chem-wisc/MetaMorpheus#2054

@bernt-matthias
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants