Spectral decon #675
Spectral decon #675
Conversation
Alexander-Sol
requested review from
nbollis,
kyp4,
acesnik,
avcarr2 and
trishorts
Codecov Report
@@ Coverage Diff @@
## master #675 +/- ##
==========================================
- Coverage 81.12% 80.98% -0.14%
==========================================
Files 147 148 +1
Lines 25690 25762 +72
==========================================
+ Hits 20840 20863 +23
- Misses 4850 4899 +49
|
The new deconvolution method relies on generating a library of theoretical isotopic distributions for all species in a given protein database. To do so, I need access to classes inside of Proteomics (e.g. PeptideWithSetModifications) |
|
Within Proteomics folder, it appears as through some things have specific namespaces while others do not: Would it make more sense to keep the namespaces of each file the same as they were before the refactoring? This would enable continuous integration as we would not need to change any namespace references in programs that implement MzLib, but would still allow for them to be in the same project, as was required for your PR. |
|
Historically, this comes from borrowing code from https://github.com/dbaileychess/CSMSL, which has a bit more specific namespace usage. |
There was a problem hiding this comment.
Alex and I conducted an in-person review over the basics of the algorithm and checked the outputs. The outputs make sense, the deconvolution added is very extendable. It doesn't affect how MetaMorpheus currently deconvolutes thanks to @nbollis's refactoring of the deconvolution code to support a well-executed design pattern.
I think it's a good time to merge this new set of code into master so we can continue working on deconvolution while maintaining a linear-ish commit history.
There was a problem hiding this comment.
Looks good to me. You could consider going through your comments one more time and removing ones that aren't meant for perpetuity. There are also quite a few TODOs to review; should these be completed before merging this? Or are they more for future note (could use something other than TODO)?
There's also a pretty big refactor of the Proteomics namespace. Were there any changes in there that are hard to review because of moving those files?
| } | ||
| else | ||
| { | ||
| //TODO: Add some averagine bullshit here |
There was a problem hiding this comment.
Haha this one's almost worth framing
There was a problem hiding this comment.
Do you still need to add something here?
There was a problem hiding this comment.
Not on this PR. That would come in subsequent iterations
| public List<Modification> VariableModifications { get; } | ||
| public DigestionParams DigestionParams { get; } | ||
| public List<SilacLabel> SilacLabels { get; } | ||
| // TODO: convert double range to MzRange |
| @@ -79,14 +79,6 @@ public virtual string ToString(string format) | |||
| return $"[{Minimum.ToString(format, System.Globalization.CultureInfo.InvariantCulture)};{Maximum.ToString(format, System.Globalization.CultureInfo.InvariantCulture)}]"; | |||
| } | |||
|
|
|||
| /// <summary> | |||
| <file src="ThermoRawFileReader\bin\x64\Release\net6.0\ThermoFisher.CommonCore.Data.dll" target="lib\net6.0" /> | ||
| <file src="ThermoRawFileReader\bin\x64\Release\net6.0\ThermoFisher.CommonCore.MassPrecisionEstimator.dll" target="lib\net6.0" /> | ||
| <file src="ThermoRawFileReader\bin\x64\Release\net6.0\ThermoFisher.CommonCore.RawFileReader.dll" target="lib\net6.0" /> | ||
| <file src="UsefulProteomicsDatabases\bin\x64\Release\net6.0\UsefulProteomicsDatabases.dll" target="lib\net6.0" /> | ||
| <!--net6.0-windows target with mzPlot--> | ||
| <file src="mzPlot\bin\x64\Release\net6.0-windows\mzPlot.dll" target="lib\net6.0-windows7.0" /> |
There was a problem hiding this comment.
This net6.0-windows section has some net6.0 stuff in it
This is the minimal viable implementation of a new deconvolution method, spectral deconvolution.
Spectral Deconvolution works by generating theoretical isotopic distributions for each peptide/proteoform in a given library, generating isotopic envelopes for each charge state, then comparing the envelopes in each spectra to those in the library.
Note that this PR includes a significant refactor (see PR #671) that moves the Proteomics project into the MassSpectrometry project. Due to this change, loading this PR necessitates a clean and rebuild and/or a restart of visual studio.
I also added a Scorer class that implements the Strategy design pattern to score experimental spectra against theoretical spectra. I still need to write tests for this class.