glasgowcompbio / vimms

A programmable and modular LC/MS simulator in Python
MIT License
19 stars 6 forks source link

Allow Top-N controller to receive an initial list of exclusions #90

Closed joewandy closed 4 years ago

joewandy commented 4 years ago

For issue #89

The Top-N controller now accepts a parameter initial_exclusion_list. This is a list of precursor ions to be excluded during Top-N selection. The list can be obtained from e.g. a previous Top-N run.

Had to broaden the RT window when creating an exclusion item, so it could match the precursor ion in the next run https://github.com/sdrogers/vimms/compare/iss_89?expand=1#diff-c6299074495d125a6af6e704ba2434ddR170. Maybe this will help with RT drift too.

To test, please run test_TopN_controller_with_beer_chems_and_initial_exclusion_list

joewandy commented 4 years ago

It seems that changing the from_rt in

rt_lower = current_time
rt_upper = current_time + rt_tol
x = ExclusionItem(from_mz=mz_lower, to_mz=mz_upper, from_rt=rt_lower, to_rt=rt_upper)

to

rt_lower = current_time - rt_tol

broke the weight calculation in https://github.com/sdrogers/vimms/blob/iss_89/vimms/Controller/topN.py#L458-L459 (the assert failed).

weight = (rt - (self.exclusion_t_0 + x.from_rt)) / (self.rt_tol - self.exclusion_t_0)
assert weight <= 1, weight

Could you help to look into this? @sdrogers

sdrogers commented 4 years ago

Whoops - breaks excluding topN controller. Can we: Give an excluded item the time it was created as well? And then use that in place of the from time at the point where the excluding topN is failing?

sdrogers commented 4 years ago

More info: the problem is that the excluding top N needs the time at which the fragmentation happened, which is how things used to be.

Should be easy to fix: if we always store excluded items with a from time of the DEW value, then we can add it to any use of from time in the excluding controller

sdrogers commented 4 years ago

Fixes #89