Fragments analysis - in source fragments #1964
Conversation
| totalItems = featureLists.stream().mapToInt(FeatureList::getNumberOfRows).sum(); | ||
| // Get parameter values for easier use | ||
| outFile = parameters.getValue(FragmentsAnalysisParameters.outFile); | ||
| // TODO |
There was a problem hiding this comment.
This is not implemented for now. Will do it later eventually, also see https://github.com/mzmine/mzmine/pull/1964/files#diff-302c9a6156e19222d2309f6673d172b2d3a64fd68daca1cdaa453b7e4bd5991bR134-R143 and https://github.com/mzmine/mzmine/pull/1964/files#diff-302c9a6156e19222d2309f6673d172b2d3a64fd68daca1cdaa453b7e4bd5991bR169-R187
|
MGF export is actually not fixed, was just random luck. |
|
Use records for dataclasses
|
|
|
||
| public record GroupedSignalScans(io.github.mzmine.datamodel.features.FeatureListRow row, | ||
| java.util.List<io.github.mzmine.datamodel.Scan> ms1Scans, | ||
| java.util.List<io.github.mzmine.datamodel.Scan> ms2Scans) { |
There was a problem hiding this comment.
here you should see an option for replace qith qualified import or similar to just import java.util.List instead of writing the whole class path
| public @NotNull Task createTask(final @NotNull MZmineProject project, | ||
| final @NotNull ParameterSet parameters, final @NotNull Instant moduleCallDate, | ||
| final @Nullable MemoryMapStorage storage, final @NotNull FeatureList featureList) { | ||
| return new io.github.mzmine.modules.dataprocessing.process_signalsanalysis.SignalsAnalysisTask( |
There was a problem hiding this comment.
same here should import io.github.mzmine.modules.dataprocessing.process_signalsanalysis.SignalsAnalysisTask
| private final double ms1Intensity; | ||
| private final double commonIntensity; | ||
|
|
||
| private SignalsResults(int commonCount, int ms1Count, int ms2Count, int precursorsCount, |
There was a problem hiding this comment.
You can use record for this instead of class. record is Java equivalent of pythons @DataClass.
It comes with default constructor and immutibility.
| * @return A set of unique dataPoints. | ||
| */ | ||
| private Set<DataPoint> collectUniqueSignalsFromScans(List<Scan> scans, MZTolerance tolerance) { | ||
| Set<DataPoint> uniqueSignals = new HashSet<>(); |
There was a problem hiding this comment.
So the unique signals here are not really unique because you still got each signal for each scan duplicated? Is that intended?
There was a problem hiding this comment.
You are right, addressed it using your version in 93b4fc2
|
@robinschmid Thank you for all the very much appreciated guidance! I tried to address your comments and added back (and renamed for clarity) the number of fragmented MS1 signals, as I believe this might also be a good metric. The |
Adafede
force-pushed
the
fragments_analysis
branch
from
f1a1768 to
08dade1
Compare
|
|
|
🚀 |
SteffenHeu
left a comment
SteffenHeu
left a comment
There was a problem hiding this comment.
Hey adriano,
this looks like a great tool, good work! I have some questions regarding the implementation though, because i sometimes dont really get whats happening. can you clarify them please?
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| /** | ||
| * Flag if signal is most likely in source fragment |
There was a problem hiding this comment.
Where/how is this flag determined? can you add a link to the method?
| import io.github.mzmine.datamodel.features.types.numbers.abstr.PercentType; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| public class Ms1SharedIntensityPercentType extends PercentType { |
There was a problem hiding this comment.
Please describe shortly here what this type does. Also we will need a documentation page. Please add a description to all the types so we know if and for what we can reuse them in the future
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| /** | ||
| * All scans of all precursors in mz and RT range |
There was a problem hiding this comment.
Across all files or in a single file?
There was a problem hiding this comment.
The one's associated to the row? Isn't this decided previously already?
| * How many of the MS1 signals in the corresponding MS1 scans were fragmented? Measured by their | ||
| * intensity | ||
| */ | ||
| public class PrecursorIonsIntensityPercentType extends PercentType { |
There was a problem hiding this comment.
so if i look at a single scan and determine how much of all ions in this scan are associated with an ms2 in a reasonable rt range?
| import io.github.mzmine.datamodel.features.types.numbers.abstr.IntegerType; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| public class SharedSignalsType extends IntegerType { |
There was a problem hiding this comment.
In an MS2? Shared between what`?
| ms2Scans); | ||
| Set<Double> precursorMzSet = ms1SignalMatchesMs2Precursors.stream().map(UniqueSignal::mz) | ||
| .collect(Collectors.toSet()); | ||
| List<UniqueSignal> ms1FragmentedSignalMatchesMs2 = ms1Signals.stream() |
There was a problem hiding this comment.
This step is based on the assumption that the unique ms1Signals are from the same file as the precursorMzSet. Otherwise the mzs may be within the tolerance, but not exactly the same. Do you need the precursorMzSet? Isnt it better to maybe filter down the ms1Signals map to a smaller range map of the unique precursors?
What is going on here exactly? Are you looking for signals that were fragmented and where the precursor mz is still in one of the ms2s?
There was a problem hiding this comment.
Edit down below it sounds like this should be all the ms1 signals that were also seen in any of the ms2 scans. Then the first filter step with the precursorMzSet prevents that from happening, or?
| // you could build a RangeMap<Double, UniqueSignal> of all rows with fragment spectra (precursor m/z) | ||
| // before looping over all rows and pass it into this method | ||
| // Or maybe we need to accumulate all MS2 fragment signals over all scans? | ||
| List<UniqueSignal> ms1SignalMatchesMs2Precursors = filterUniquePrecursors(ms1SignalMap, |
There was a problem hiding this comment.
naming proposal: uniqueMs1SignalWithMs2Scans? sounds more understable
| double ms1SignalsMatchedPercent = signalsMatched / (double) ms1SignalsTotal; | ||
|
|
||
| // for all precursor ions | ||
| int ms2SignalsAllPrecursors = mapToList(ms2SignalMap).size(); |
There was a problem hiding this comment.
doesnt RangeMap have a size method?
|
|
||
| // only for this precursor ion | ||
| List<UniqueSignal> ms2Signals = mapToList(ms2SignalMap).stream() | ||
| .filter(signal -> originatesFromPrecursorIon(signal, tolerance, precursorMz)).toList(); |
There was a problem hiding this comment.
i dont really get it here, what is this doing? It's going through every ms2 signal, then going to the ms2 scans of these signals, getting the precursor mz and then checking if the scan's precursor mz matches the precursorMz?
Then the easier way is to just check the precursor mz of the ms2 scans themselves or? In the current way each scan is checked as much times as it has ms2 signals or not?
There was a problem hiding this comment.
originatesFromPrecursorIon sounds more like "does this signal appear in the ms2 of this precursor" to me but then you would need to compare the signals in the ms2s of that precursor to the signals in the ms2Signal map. Or is that happening here and i dont get it? :D sorry
| * @param precursorMz | ||
| * @return The results of the signal count. | ||
| */ | ||
| private SignalsAnalysisResult countUniqueSignalsBetweenMs1AndMs2(List<Scan> ms1Scans, |
There was a problem hiding this comment.
this method is doing a lot of things and is quite hard to understand. is it possible to divide it into smaller chunks?
Adafede
force-pushed
the
fragments_analysis
branch
from
351453a to
cdda0a9
Compare
Adafede
force-pushed
the
fragments_analysis
branch
4 times, most recently
from
554b385 to
e822060
Compare
# Conflicts: # mzmine-community/src/main/java/io/github/mzmine/datamodel/data_access/EfficientDataAccess.java # mzmine-community/src/main/java/io/github/mzmine/gui/mainwindow/MainMenu.fxml # mzmine-community/src/main/java/io/github/mzmine/modules/batchmode/BatchModeModulesList.java # mzmine-community/src/main/java/io/github/mzmine/modules/dataprocessing/filter_isotopefinder/IsotopeFinderTask.java # mzmine-community/src/main/java/io/github/mzmine/util/scans/ScanUtils.java
…nd later use (also, skipping first isotope)
3 participants
No description provided.