Skip to content

Fix tests ii#810

Merged
mschwoer merged 6 commits intomainfrom
fix_tests_II
Apr 14, 2026
Merged

Fix tests ii#810
mschwoer merged 6 commits intomainfrom
fix_tests_II

Conversation

@mschwoer
Copy link
Copy Markdown
Collaborator

@mschwoer mschwoer commented Apr 13, 2026

Fix tests (detected only by the "full matrix"

@GeorgWa not sure why this did not come up earlier? b1a13fa

@mschwoer mschwoer requested a review from GeorgWa April 13, 2026 15:21
Comment thread alphadia/libtransform/prediction.py Outdated
process_num = self.mp_process_num
if device == "mps" and process_num > 1:
logger.info(
"Multiprocessing is not supported with MPS device, falling back to single process"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I’m not aware of this issue tbh. Do we need this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the tests tell us so? but I am also confused

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it was introduced by 5a8920a

  reinitialize_ms2_model creates a new pDeepModel() without passing the device — so it defaults to CPU. After the call:
  - ms2_model.device_type is "cpu"
  - peptdeep's guard self.ms2_model.device_type != "cpu" evaluates to False
  - It falls through to the multiprocessing path
  - But the other models (RT, CCS) are still on MPS, and share_memory_() fails on those MPS tensors

I patched it here at the two places where reinitialize_ms2_model is called .. but peptdeep should preserve the device I guess ..

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

so can we solve this on the peptdeep level?

Copy link
Copy Markdown
Collaborator

@GeorgWa GeorgWa left a comment

Choose a reason for hiding this comment

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

Looks good

@mschwoer mschwoer merged commit 00dca3b into main Apr 14, 2026
28 checks passed
@mschwoer mschwoer deleted the fix_tests_II branch April 14, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants