Skip to content
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

Fix for "A duplicate key has already been added" exception during chromatogram extraction… #1659

Conversation

bspratt
Copy link
Member

@bspratt bspratt commented Jun 28, 2021

… when two molecules in different molecule lists have identical precursor but different fragments.

Reported by user Anna Illiano.

…omatogram extraction when two molecules in different molecule lists have identical precursor but different fragments.

Reported by user Anna Illiano.
@nickshulman
Copy link
Contributor

Currently, the difference between "AreEquivalentGroupNodes" and "AreEquivalentGroups" is that the "AreEquivalentGroupNodes" also calls "EquivalentChildren".

With this change, "AreEquivalentGroups" also calls "EquivalentChildren" if they're non-proteomic.

I wonder if this means that we should combine "AreEquivalentGroupNodes" and "AreEquivalentGroups". I do not understand when someone would need to call one or the other.

(I don't understand this code very well at all-- this is just something I noticed)

@brendanx67
Copy link
Contributor

Currently, the difference between "AreEquivalentGroupNodes" and "AreEquivalentGroups" is that the "AreEquivalentGroupNodes" also calls "EquivalentChildren".

With this change, "AreEquivalentGroups" also calls "EquivalentChildren" if they're non-proteomic.

I wonder if this means that we should combine "AreEquivalentGroupNodes" and "AreEquivalentGroups". I do not understand when someone would need to call one or the other.

(I don't understand this code very well at all-- this is just something I noticed)

@brendanx67 brendanx67 closed this Jun 28, 2021
@brendanx67
Copy link
Contributor

Oops

@brendanx67 brendanx67 reopened this Jun 28, 2021
@brendanx67
Copy link
Contributor

This looks like a really odd change to me. The original AreEquivalentGroups function appears to be targeted at deciding whether to precursors are equivalent within the same molecule/peptide, i.e. they share an adduct and label type. Those are the only two distinguishing features between two precursors of the same molecule.

I am really not clear on why this would cause chromatogram extraction to throw an exception. But, perhaps the function is getting misused somehow or overly overloaded from its original purpose.

@bspratt
Copy link
Member Author

bspratt commented Jun 28, 2021

I guess this is old enough SRM handling code that none of us wants to claim responsibility for it. But if you run the updated test without the changed AreEquivalentGroups you can quickly arrive at the problem, which as I understand it is that two groups are being "merged" even though they have dissimilar children. Near as I can tell that was never a problem with peptides since the children are generated and can't help but be the same.

@nickshulman
Copy link
Contributor

I believe this recent support request also touches on some of the same issues:
https://skyline.ms/announcements/home/support/thread.view?rowId=52220

Things in the .skyd file are identified by the peptide/molecule text id plus the precursor m/z.
If two different precursors in the Document happen to have the same key, then their chromatograms get stored in the same place in the .skyd file, and weird things may result.

I wonder if we should add stuff to the .skyd file so that two different peptides or molecules from the document are guaranteed to have separate keys in the .skdy file.

@bspratt
Copy link
Member Author

bspratt commented Oct 4, 2021

Closing this, Nick is going to take a different approach to the problem

@bspratt bspratt closed this Oct 4, 2021
@bspratt bspratt deleted the Skyline/work/20210628_fix_duplicate_small_mol_precursors_issue branch October 4, 2021 23:50
@nickshulman nickshulman restored the Skyline/work/20210628_fix_duplicate_small_mol_precursors_issue branch October 9, 2021 16:31
@brendanx67 brendanx67 deleted the Skyline/work/20210628_fix_duplicate_small_mol_precursors_issue branch January 10, 2022 21:23
@brendanx67 brendanx67 restored the Skyline/work/20210628_fix_duplicate_small_mol_precursors_issue branch January 10, 2022 21:23
@bspratt bspratt deleted the Skyline/work/20210628_fix_duplicate_small_mol_precursors_issue branch January 10, 2022 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants