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

Asking for 0 isotopes causes R crash #20

Open
wkumler opened this issue Mar 28, 2020 · 3 comments
Open

Asking for 0 isotopes causes R crash #20

wkumler opened this issue Mar 28, 2020 · 3 comments

Comments

@wkumler
Copy link

wkumler commented Mar 28, 2020

Hi, awesome package! I've been using it for a while and just now tried to use it to identify MSMS fragment formulas, which means I don't need the isotope data. However, when I set maxisotopes=0, it causes R to crash.

E.g.:

library(Rdisop)
decomposeMass(mass = 147.0529, maxisotopes = 0)

It's not critically important because I can just ignore the isotope data, but took me while to debug and returning an error or warning instead would be super helpful.

@janlisec
Copy link
Contributor

janlisec commented Nov 5, 2024

Hi @wkumler, I would implement a patch checking the maxisotopes parameter in R and return a warning, if specified as '0' instead of causing a crash.

However, I understood that your intention was to have the isotopes part of the function result to be omitted, correct? Why would you prefer this behavior?

@wkumler
Copy link
Author

wkumler commented Nov 5, 2024

I mostly use Rdisop for mass decomposition, not isotope envelope prediction. My thought was that the algorithm could be sped up and the output cleaned up if the option to disable isotope envelope prediction was disabled. This is especially relevant for isotope-labeled experiments where the predictions aren't useful because I'm not working with natural abundances. So yes, while an error or warning thrown upon maxisotopes = 0 would be useful (and would have saved me an hour or two of frustrated debugging), a better solution would be to skip the envelope prediction part of the algorithm entirely when maxisotopes=0.

@janlisec
Copy link
Contributor

janlisec commented Nov 6, 2024

I see, thanks for the clearification. I could implement the output clean-up in R (but you probably have done this as a post processing function already). I can not exclude the calculation from the C++ functions called underneath (as I am not fluent in C++). So excluding the isotope information would not speed up the function.

However, I actually use Rdisop because it is really fast already (compared to enviPat as the best alternative I am aware of). Still, a reasonable parameter setting should not lead to a crash and this will be fixed shortly.

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

No branches or pull requests

2 participants