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

added hyperscore calculation to PEP #2360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trishorts
Copy link
Contributor

No description provided.

@trishorts trishorts added the WIP Work in progress, not ready for review label Apr 17, 2024
ulong nIonFactorial = GetFactorial((ulong)nIons.Count);
nIonFactorial = nIonFactorial < 0 ? ulong.MaxValue : nIonFactorial; // this is a hack to prevent overflow
ulong cIonFactorial = GetFactorial((ulong)cIons.Count);
cIonFactorial = cIonFactorial < 0 ? ulong.MaxValue : cIonFactorial; // this is a hack to prevent overflow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ulong will overflow after 20 fragment ions. This check will not prevent overflow due to it being unsigned. We should discuss what max value is required. BigInteger does not appear to have a practical limit to its value.
image

@trishorts
Copy link
Contributor Author

I think it goes a bit higher. Nonetheless it is not meant to prevent overflow. When overflow happens a negative number is returned. This check will substitute ulong max for that negative number The answer won't be correct but it will be large. In fact it will be much larger than a decoy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress, not ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants