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

Devel #220

Closed
wants to merge 7 commits into from

Conversation

Projects
None yet
3 participants
@MaliRemorker
Copy link

commented Jul 11, 2019

No description provided.

@MaliRemorker

This comment has been minimized.

Copy link
Author

commented Jul 11, 2019

Pulling in the parallel changes

@meowcat

This comment has been minimized.

Copy link

commented Jul 12, 2019

Hi,
looks nice; why did you close your pull request again?

@schymane

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

We're still testing internally, it was a commit meant for the MaliRemorker repo not yet the official RMB one - are you happy to integrate when the time is right? We didn't want to interfere too much with S4power ...

@meowcat

This comment has been minimized.

Copy link

commented Jul 12, 2019

The msmsRead interface is not going to change with s4power. I like the idea of parallelization. I was actually thinking of parallelizing the analyze and reanalyze stuff (actually at very little expense we could directly do both in one step saving us a lot of time.)

I wonder whether we actually want a separate function. Look at the pattern in https://github.com/meowcat/RMassScreening/blob/master/R/batchPick.R where I handle both cases in the same function, would this be an option?

@schymane

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

The parallelization has improved performance dramatically ... we're running thru changes in many places - also now having to merge CEs from different runs at the msmsRead stage to get enough data for recal ... (rather than originally later on just for multiplicity filtering). We can look into merging functions, if I understand @MaliRemorker is working off a very "abgespeckte" version and once we've tested it for bombproofness let's talk about integration. Keep an eye on his repo perhaps to see what's going on in the interim?

@meowcat

This comment has been minimized.

Copy link

commented Jul 12, 2019

Also, have you tested the performance? The peak picking in RMassScreening I use with up to 11 parallel threads, but that uses some weird readMzXml function, not the mzR backend (unless Martin changed it.) However the ProteoWizard guys claim that for raw to mzXML conversion, I/O becomes limiting pretty quickly if you use more than 2 threads. So I wonder where this one falls.

@meowcat

This comment has been minimized.

Copy link

commented Jul 12, 2019

also now having to merge CEs from different runs at the msmsRead stage to get enough data for recal

For this one, however, we should think more carefully at a good interface. I am actually also writing some changes (to extract EICs for all fragments, and use the EIC to filter out chemical noise a bit better) so we should try to not collide too much.

@schymane

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Yes, we are working on prescreening as well - but had not got far enough to look at EICs for fragments. There is definitely room to improve removal of noise up front using EICs so that we do not even process data for which no precursor is detected etc. Adding in fragment patterns would be even better.
Sounds like we definitely need to coordinate at some point, for now we are still playing around as we are also not yet getting optimal data from the instruments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.