-
Notifications
You must be signed in to change notification settings - Fork 152
Add MS2Deepscore to MZMine #1737
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
Conversation
|
@robinschmid Maybe a good moment for an intermediate code review. It seems to all work pretty well. The model makes predictions and they are the same as the model in python! |
|
I will look at it tomorrow. Sounds great so far. |
...rc/main/java/io/github/mzmine/util/scans/similarity/impl/ms2deepscore/MS2DeepscoreModel.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/github/mzmine/util/scans/similarity/impl/ms2deepscore/MS2DeepscoreModel.java
Outdated
Show resolved
Hide resolved
...est/java/io/github/mzmine/util/scans/similarity/impl/ms2deepscore/MS2DeepscoreModelTest.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/github/mzmine/util/scans/similarity/impl/ms2deepscore/MS2DeepscoreModel.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/github/mzmine/util/scans/similarity/impl/ms2deepscore/MS2DeepscoreModel.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/github/mzmine/util/scans/similarity/impl/ms2deepscore/MS2DeepscoreModel.java
Outdated
Show resolved
Hide resolved
robinschmid
left a comment
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.
Great so far
|
Mapping the SpectralTensors might be best done if the model can run in parallel natively but if that does not work you could do: List<SpectralTensor> list = ...
list.stream.parallel().map(tensor -> process to get new tensor).toList() This will give you a new list |
# Conflicts: # gradle/libs.versions.toml
utilities for pair processing in parallel stream
|
@robinschmid The current version has all basic functionality. MS/MS spectral networking (ms2deepscore) works. We can select a model file. I have added a test file to the resources/models so you can use this for testing (but the results will be meaningless). The larger models I tested myself also work fine. And visualization in interactive ion identity molecular networking is working. Could you let me know if everything is implemented in a correct way and what steps we should still do? |
| public static final OptionalParameter<DirectoryParameter> downloadDirectory = new OptionalParameter<>( | ||
| new DirectoryParameter("Download model directory", | ||
| "The directory in which the model is stored", | ||
| Objects.requireNonNull(FileAndPathUtil.resolveInMzmineDir("models")).toString())); |
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 should set the default to the users/.mzmine/models folder. However, I was not able to check this, since it seems to be overwritten by the last set directory. (which I did not no how to reset) So please double check if this is correct. @robinschmid
| public static final FileNameParameter ms2deepscoreModelFile = new FileNameParameter( | ||
| "MS2Deepscore model", | ||
| "The file location of the MS2Deepscore model, click download to download the model.", | ||
| FileSelectionType.OPEN, true); |
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.
It would be ideal if this field is updated automatically when we set the download folder. Or at least that the last folder is updated to reflect the location of the downloaded files. Is something like that easily doable?
| "The file location of the MS2Deepscore model, click download to download the model.", | ||
| FileSelectionType.OPEN, true); | ||
|
|
||
| public static final FileNameParameter ms2deepscoreSettingsFile = new FileNameParameter( |
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.
I gave the user now has a lot of options. Like where to store the downloaded models, or to use a different model etc. An alternative is to remove that option altogether and just always downloading the model to a predefined location. What do you think is most user friendly? @robinschmid
|
@robinschmid @ansgarkorf Everything is working now. There is an option to download a ms2deepscore model in the module, predictions are made and it can be visualized in the network overview. I left a few comments regarding steps that could be improved for setting the parameters, but I didn't know how to implement them. One more thing, I did not build in any check that it can only run on LC/MSMS data. Could one of you add this maybe? I will be on holidays for about a month. So feel free to make changes and/or merge the PR if you feel like it is ready for release. |
|
Thank you for all the work @niekdejonge and enjoy your holiday! Did @robinschmid and you discuss why MS2Deepscore does not implement the SpectralSimilarityFunction interface so we can use it everywhere where we use cosine similarity? |
|
we have to check it. And see if we can come up with a good strategy for this. Not sure if the interface design is good. |
|
The input takes a parameter set, so i think that should not be a problem, or? |
|
MS2Deepscore works so differently that it does not really fit in. No mz tolerance, no minimum matched signals. public abstract SpectralSimilarity getSimilarity(ParameterSet parameters, MZTolerance mzTol,
int minMatch, DataPoint[] library, DataPoint[] query);Best is to send a batch of spectral data to calculate the embeddings - then the similarity is a simple vector comparison by cosine sim. So the spectral alignment step is not needed. I think it might be good to have two interfaces somehow - one for vector based and one for spectral based. |
Adding a module to MZMine that is able to make ms2deepscore (https://github.com/matchms/ms2deepscore) predictions.