OpenMS icon indicating copy to clipboard operation
OpenMS copied to clipboard

add MSGF+ allowDenseCentroidedPeaks parameter

Open bernt-matthias opened this issue 2 years ago • 4 comments

Description

  • [ ] add MSGF+ -allowDenseCentroidedPeaks parameter
  • [ ] add test
  • [ ] always output MSGF+ commandline, stdout and stderr in case of error?

Checklist

  • [ ] Make sure that you are listed in the AUTHORS file
  • [ ] Add relevant changes and new features to the CHANGELOG file
  • [ ] I have commented my code, particularly in hard-to-understand areas
  • [ ] New and existing unit tests pass locally with my changes
  • [ ] Updated or added python bindings for changed or new classes (Tick if no updates were necessary.)

How can I get additional information on failed tests during CI

Click to expand If your PR is failing you can check out
  • The details of the action statuses at the end of the PR or the "Checks" tab.
  • http://cdash.openms.de/index.php?project=OpenMS and look for your PR. Use the "Show filters" capability on the top right to search for your PR number. If you click in the column that lists the failed tests you will get detailed error messages.

Advanced commands (admins / reviewer only)

Click to expand
  • /reformat (experimental) applies the clang-format style changes as additional commit. Note: your branch must have a different name (e.g., yourrepo:feature/XYZ) than the receiving branch (e.g., OpenMS:develop). Otherwise, reformat fails to push.
  • setting the label "NoJenkins" will skip tests for this PR on jenkins (saves resources e.g., on edits that do not affect tests)
  • commenting with rebuild jenkins will retrigger Jenkins-based CI builds

:warning: Note: Once you opened a PR try to minimize the number of pushes to it as every push will trigger CI (automated builds and test) and is rather heavy on our infrastructure (e.g., if several pushes per day are performed).

bernt-matthias avatar Jan 18 '23 13:01 bernt-matthias

You have to decide if you want the param with underscores or camelCase. But it needs to be consistent!

jpfeuffer avatar Jan 19 '23 20:01 jpfeuffer

I think you made the right choice with underscores but it needs to be with underscores in the test as well.

jpfeuffer avatar Jan 19 '23 20:01 jpfeuffer

@jpfeuffer user provided me a ~3MB test file that could not be parsed before. Would this be fine?

Is there a way to get the binaries that are generated by the CI for this PR? Could you remind me where I find the compilation instructions if not.

bernt-matthias avatar Jan 20 '23 17:01 bernt-matthias

If possible, try to run a FileFilter and remove any MS1 spectra and filter for a few MS2 spectra that would fail without the flag. It is on the high end.

Just develop on https://gitpod.io#github.com/OpenMS/OpenMS/pull/6606 . It will be (mostly) precompiled. Change back the files to separate files (i.e. just copy MSGFPlus_1_out)! Otherwise, you will overwrite. Run ctest -R MSGFPlusAdapter_2 to check. And ./tools/overwriteTOPPtests.sh MSGFPlusAdapter_2 to overwrite the "groundtruth test files" with the output from the current test run.

jpfeuffer avatar Jan 20 '23 17:01 jpfeuffer