Skip to content

Fix chem-msms-predict with public MassSpecGym checkpoints (instrument arg + ms_pred patches)#38

Merged
bowen-bd merged 7 commits into
learningmatter-mit:mainfrom
hugogontijomachado:fix/iceberg-public-checkpoints
Jun 12, 2026
Merged

Fix chem-msms-predict with public MassSpecGym checkpoints (instrument arg + ms_pred patches)#38
bowen-bd merged 7 commits into
learningmatter-mit:mainfrom
hugogontijomachado:fix/iceberg-public-checkpoints

Conversation

@hugogontijomachado

Copy link
Copy Markdown
Contributor

Summary

Make chem-msms-predict (ICEBERG forward prediction) actually run with the public
MassSpecGym checkpoints
on a standard install. Two changes:

1. predict_msms.py: add --instrument (default Orbitrap)

The public MassSpecGym ICEBERG checkpoints condition on an instrument feature
(Orbitrap | QTOF). The skill never passed one, so instrument defaulted to None,
was serialized to the candidates TSV as an empty cell, read back as NaN, and crashed the
generator with RuntimeError: value cannot be converted to type int64 without overflow.
Added an --instrument argument (default Orbitrap) threaded into iceberg_prediction.

2. conda-envs/msms-agent/install.sh: patch 3 upstream ms_pred bugs after clone

With ms_pred pip-installed and run on CPU with pytorch_lightning ≥ 2.0, ICEBERG forward
prediction hits three upstream bugs (none macOS-specific):

  • iceberg_elucidation.py invokes predict_smis.py via a cwd-relative path (only
    resolves from a repo checkout).
  • predict_smis.py uses pl.utilities.seed.seed_everything (removed in PL 2.0).
  • predict_smis.py calls torch.cuda.set_device(gpu_id) unconditionally (fails on CPU).

A post-clone patch step applies the three fixes so the env is usable out of the box.

Note: the install.sh patch is a temporary bridge. I submitted the same three fixes
upstream — coleygroup/ms-pred#35. Once that merges, this patch step can be dropped
(or replaced by bumping the ms_pred pin).

Testing

Verified end-to-end on macOS arm64 (CPU): glyphosate MS/MS for [M-H]- (precursor
168.0067) and [M+H]+ (170.0213) at CE 10/20/40/60 eV produce sensible spectra
(168→124, 170→88, 170→124, water losses).

hugogontijomachado and others added 5 commits June 11, 2026 12:58
…nstalled ms_pred / CPU / PL 2.x

- predict_msms.py: add --instrument (default Orbitrap). The public MassSpecGym
  checkpoints condition on an instrument (Orbitrap|QTOF); it previously defaulted
  to None -> serialized to NaN -> int64 overflow crash in the gen model.
- install.sh: patch upstream ms_pred after clone for 3 bugs that break ICEBERG
  forward prediction when ms_pred is pip-installed / run on CPU / with PL 2.x:
    * iceberg_elucidation.py invoked predict_smis.py via a cwd-relative path
    * predict_smis.py used pl.utilities.seed.seed_everything (removed in PL 2.0)
    * predict_smis.py called torch.cuda.set_device(gpu_id) unconditionally (CPU-fail)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@mlederbauer mlederbauer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi! Creator of this skill here. Thank you very much for this addition that enables a user to run ICEBERG on CPU, and the missing instrument flag. I am kindly asking to merge PR hugogontijomachado#1 into your feature branch with some additional edits, after which this branch id ready to merge into main.

conda run -n ms-gen python .agents/skills/chem-msms-predict/scripts/predict_msms.py \
    --smiles "c1ccccc1C(=O)OCCN" \
    --gen_ckpt downloads/iceberg_dag_gen_msg_best.ckpt \
    --inten_ckpt downloads/iceberg_dag_inten_msg_best.ckpt \
    --collision_energies 20 40 \
    --adduct "[M+H]+" \
    --instrument "Orbitrap" \
    --output_dir results/msms_prediction
Image

Thank you very much! cc @bowen-bd

@bowen-bd

Copy link
Copy Markdown
Collaborator

Thanks @hugogontijomachado for the contribution and @mlederbauer for the review!
Can @hugogontijomachado please let us know when @mlederbauer's PR has been merged into this PR?

hugogontijomachado and others added 2 commits June 12, 2026 11:59
…ally runs

Verified by rebuilding the env from install.sh on macOS arm64: the build succeeds,
but ICEBERG forward prediction crashes at runtime with unpinned `ray`:
  - `ray` (no `[tune]` extra) is missing pyarrow -> ModuleNotFoundError: pyarrow
  - it resolves ray>=2.8, which removed `ray.tune.integration.pytorch_lightning`
    that ms_pred imports eagerly -> "Can't import ray.tune"
Pinning ray[tune]==2.7.2 brings the tune extra + the integration module; ray 2.7.2
imports pkg_resources, removed in setuptools 81, so also pin setuptools<81.
After this, predict_msms.py runs end-to-end (glyphosate [M-H]- -> 168.0067, spectrum).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hugogontijomachado

Copy link
Copy Markdown
Contributor Author

Thanks @mlederbauer for the review and @bowen-bd for taking a look!

@bowen-bd — answering your question: @mlederbauer's PR (hugogontijomachado#1) is now merged into this branch.

Before merging I rebuilt the env from the updated install.sh on macOS (arm64) in a throwaway env to verify. The new macOS path (torch 2.3.0 + DGL-torch2.3 + torchdata stub) builds cleanly — and is actually nicer on arm64 than pinning torch 2.2.2, since PyG ships universal2 torch-scatter/torch-sparse wheels for 2.3.0 (no source build needed).

The rebuild surfaced one runtime issue: with ray unpinned in core_env.yaml, the env resolves ray 2.55 without the tune extra, so predict_smis.py crashes at runtime — ModuleNotFoundError: pyarrow and Can't import ray.tune (ms_pred imports ray.tune.integration.pytorch_lightning, removed in ray ≥ 2.8). I pushed a small follow-up pinning ray[tune]==2.7.2 (+ setuptools<81, since ray 2.7.2 still imports pkg_resources). After that, ICEBERG forward prediction runs end-to-end (glyphosate [M-H]- → precursor 168.0067, spectrum produced).

Branch should be ready to merge into main — thanks both!

@bowen-bd bowen-bd merged commit 66f4cff into learningmatter-mit:main Jun 12, 2026
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

Successfully merging this pull request may close these issues.

3 participants