eugenemel/maven

Peaks Search not respecting MS2 score threshold

Closed this issue · 4 comments

Metabolomics Search, Diff Iso Search, X0202 M003B set, retain unknowns, MS2 threshold to 0.7 (so, anything under this threshold should be demoted to an unknown)

As it turns out, this symptomatic, of a much bigger problem - the SearchParameters base class has a method, baseParams(), which sets a bunch of parameters. I was calling this in encodeParameters(), but this actually doesn't work, as it will always encode or decode the default values from SearchParameters instead of any overwritten values.

For this reason, all of parameter encoding/decoding needs to be cleaned up to not use this system. Essentially, any attempts to override a base class parameter, then carry out encoding/decoding, would have been failing (using the incorrect value). This has major implications for lipidomics, but may also affect metabolomics work, depending on where pipelines are used.

After some thought and conversations with gemini, it appears that using inheritance at all here is just a bad idea. Instead, just switch everything to composition, and always access through some base set of fields. Or, just copy everything around and forget it.

For example, get rid of the baseParams() function entirely (or don't use it for encoding), and just reimplement everything in encodeParams()

After some experimentation, figured out that the secret is to refactor baseParams() and fillInBaseParams() into a template class, templatized with the derived class, and then call the derived class fields in the template. This ensures that the derived class field value is always used (never the base class values). It took a while for chatGPT to understand that this was was needed.

This pattern is known as "curiously recurring template pattern" CRTP, e.g. class Derived : public Base<Derived>. This issue is actually exactly what the pattern is meant to address!