Share the modification registry with multiprocessing workers - #428
Open
GeorgWa wants to merge 4 commits into
Open
Share the modification registry with multiprocessing workers#428GeorgWa wants to merge 4 commits into
GeorgWa wants to merge 4 commits into
Conversation
Workers are started with the "spawn" start method and re-import alphabase, so they only ever saw the modifications in modification.tsv. Every runtime change to the registry -- custom modifications, modloss filtering, lower-case AAs, a custom TSV -- was silently absent in workers, raising KeyError on mod lookups. MOD_DF is the single source of truth and update_all_by_MOD_DF() already rebuilds everything derived from it, so a copy of MOD_DF is a complete snapshot of the registry. get_modification_state()/set_modification_state() move that snapshot between processes, and spawn_pool() installs it in each worker as it starts. Because the snapshot is the whole table rather than a record of which mutators ran, it stays correct as new mutators are added. This replaces the fallback in SpecLibBase.calc_precursor_isotope_intensity, which silently dropped to a single process whenever custom modifications were present -- and which never covered the other three mutators anyway, since has_custom_mods() only tests for the User-added classification. has_custom_mods itself stays exported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four pools had the same shape -- batchify, imap, optional progress bar, concat -- reimplemented each time, with three separate batching helpers between them. parallel_imap() and parallel_apply() carry that shape once, so callers no longer construct a pool, choose a context, or think about what state a worker needs; spawn_pool is now private. Public signatures are unchanged. The helpers this removes were all private: _batchify_df (defined twice, with different signatures), _batchify_series and _count_batchify_df. Fixes two progress bar bugs found on the way: SpecLibBase.calc_precursor_isotope_info() passed process_bar= to a parameter named progress_bar=, so the multiprocessing branch raised TypeError and was dead. It also dropped mp_batch_size, which is now forwarded. calc_precursor_isotope_intensity_mp() documented progress_bar as a flag but peptdeep passes a callback. A callable was truthy, so it fell into the tqdm branch and the callback was silently discarded. Progress now accepts True for a tqdm bar, a callable to supply your own, or anything falsy for none, which also restores the callback contract calc_precursor_isotope_info_mp already had. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "not full" install has no numba. Three tests call isotope functions that need it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mschwoer
reviewed
Sep 3, 2026
| return len(MOD_DF[MOD_DF["classification"] == _MOD_CLASSIFICATION_USER_ADDED]) > 0 | ||
|
|
||
|
|
||
| def get_modification_state() -> pd.DataFrame: |
Contributor
There was a problem hiding this comment.
maybe make explcit that these methods target multiprocessing ..
| return ctx.Pool( | ||
| processes, | ||
| initializer=set_modification_state, | ||
| initargs=(get_modification_state(),), |
Contributor
There was a problem hiding this comment.
do I get correctly that get_modification_state() reads the MOD_DF from the main threads, and set_modification_state() gets it as input arg on the spawned threads?
| """ | ||
| global MOD_DF | ||
| MOD_DF = mod_df | ||
| update_all_by_MOD_DF() |
Contributor
There was a problem hiding this comment.
this mutates MOD_INFO_DICT, MOD_CHEM, etc .. does this work as intended on the worker threads?
|
|
||
|
|
||
| def _batch_count(obj, batch_size: int, group_by=None) -> int: | ||
| sizes = obj.groupby(group_by).size().values if group_by else [len(obj)] |
Contributor
There was a problem hiding this comment.
here we duplicate logic of _batchify .. could this be returned by _batchify>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A worker starts with
spawnand imports alphabase again. It has only the modifications frommodification.tsv. Run-time changes are absent: custom modifications, modloss filtering, lower-case AAs, a custom TSV.calc_precursor_isotope_intensityused a fallback to one process. The other multiprocessing functions used the incorrect registry and gave no warning.Change
_spawn_pool()copiesMOD_DFin the parent and installs it in each worker, withget_modification_state()andset_modification_state(). The fallback andhas_custom_mods()are removed.The copy is complete, so a new column reaches the workers without a change here. #303 sent selected fields and thus needed a change for each new field.
parallel_imap()andparallel_apply()inalphabase/utils.pyare now the only functions that make a pool. Converted:calc_precursor_isotope_info_mp,calc_precursor_isotope_intensity_mp,_apply_translate_modifications_mp,SpecLibDecoy.translate_to_decoy. This removes three copies of_batchify_dfand 125 lines.Also corrects
calc_precursor_isotope_info, which sentprocess_bar=to a parameter with the nameprogress_barand thus raisedTypeError.Tests
test_worker_registry_matches_parentcompares the registry of the worker with the registry of the parent, after each of four run-time changes. It fails without this PR.