fix(eval): serialize synthetic CUDA batch jobs

[manzuoni-astera]

Summary

• clamp process-based synthetic generation to one worker on CUDA
• preserve requested joblib parallelism for CPU execution
• apply the shared guard to both density and structure-factor batch paths

Closes #242

Validation

• pytest tests/eval/test_synthetic_utils.py -q (10 passed)
• pytest tests/eval/test_generate_synthetic_density.py tests/eval/test_generate_synthetic_sf.py -q (14 passed)
• ty check src/sampleworks/eval/synthetic_utils.py src/sampleworks/eval/generate_synthetic_density.py src/sampleworks/eval/generate_synthetic_sf.py tests/eval/test_synthetic_utils.py
• ruff check and ruff format --check on touched files

Summary by CodeRabbit

• Bug Fixes

  • Improved synthetic data generation on CUDA devices by automatically limiting parallel processing when multiple workers could cause memory contention.
  • Preserved requested parallelism for CPU-based processing and single-worker CUDA jobs.
  • Updated processing logs to report the effective worker count.

• Tests

  • Added coverage for CUDA worker limits, CPU parallelism, and single-worker CUDA execution.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds resolve_parallel_jobs to clamp unsafe CUDA parallelism to one worker, preserves requested CPU parallelism, and applies the resolved count in synthetic density and structure-factor batch processing.

Changes

CUDA parallelism control

Layer / File(s)	Summary
Parallel job resolution and tests src/sampleworks/eval/synthetic_utils.py, tests/eval/test_synthetic_utils.py	Adds resolve_parallel_jobs, which clamps negative or multi-worker CUDA requests to 1 while preserving other values, with pytest coverage for CUDA and CPU cases.
Batch processing integration src/sampleworks/eval/generate_synthetic_density.py, src/sampleworks/eval/generate_synthetic_sf.py	Both batch processors resolve n_jobs, log the effective count, and pass it to joblib.Parallel.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• diff-use/sampleworks#234: Introduced the batch parallelism handling that this change updates.

Suggested reviewers: marcuscollins, copilot, dorismai, k-chrispens

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: serializing synthetic CUDA batch jobs.
Linked Issues check	✅ Passed	The PR clamps CUDA batch parallelism to one worker and preserves CPU parallelism, matching issue #242.
Out of Scope Changes check	✅ Passed	The changes are limited to the shared CUDA job guard, its call sites, and targeted tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR addresses GPU memory contention when generating synthetic structure factors or density maps with joblib multiprocessing by clamping CUDA batch execution to a single worker, while preserving the caller’s requested parallelism for CPU execution.

Changes:

• Add a shared resolve_parallel_jobs() utility that warns and forces n_jobs=1 for CUDA when the request would imply multiple processes.
• Apply the shared guard to both synthetic structure-factor and synthetic density batch processing paths.
• Add focused unit tests covering CUDA clamping and CPU pass-through behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
tests/eval/test_synthetic_utils.py	Adds unit tests validating CUDA serialization and CPU parallelism preservation.
src/sampleworks/eval/synthetic_utils.py	Introduces resolve_parallel_jobs() with a warning + clamp behavior for CUDA multiprocessing.
src/sampleworks/eval/generate_synthetic_sf.py	Uses the shared guard to compute effective_n_jobs for SF batch processing.
src/sampleworks/eval/generate_synthetic_density.py	Uses the shared guard to compute effective_n_jobs for density batch processing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

🧹 Nitpick comments (1)

src/sampleworks/eval/generate_synthetic_density.py (1)

274-275: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

n_jobs docstring doesn't mention CUDA clamping in both process_batch functions. Both process_batch docstrings describe n_jobs as "Number of parallel jobs. -1 means use all available CPUs." but don't mention that resolve_parallel_jobs now clamps to 1 on CUDA devices. Adding a brief note would help users understand the effective behavior.

• src/sampleworks/eval/generate_synthetic_density.py#L274-L275: Add a note about CUDA clamping to the n_jobs parameter description.
• src/sampleworks/eval/generate_synthetic_sf.py#L460-L461: Apply the same docstring update.

📝 Suggested docstring update (apply to both files)

     n_jobs
         Number of parallel jobs. -1 means use all available CPUs.
+        On CUDA devices, n_jobs is automatically limited to 1 to prevent
+        GPU memory contention from multiple CUDA contexts.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/sampleworks/eval/generate_synthetic_density.py` around lines 274 - 275,
Update the n_jobs parameter descriptions in both process_batch functions to
document that resolve_parallel_jobs clamps the effective job count to 1 when
running on CUDA devices. Apply the same concise clarification in
src/sampleworks/eval/generate_synthetic_density.py lines 274-275 and
src/sampleworks/eval/generate_synthetic_sf.py lines 460-461.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/sampleworks/eval/generate_synthetic_density.py`:
- Around line 274-275: Update the n_jobs parameter descriptions in both
process_batch functions to document that resolve_parallel_jobs clamps the
effective job count to 1 when running on CUDA devices. Apply the same concise
clarification in src/sampleworks/eval/generate_synthetic_density.py lines
274-275 and src/sampleworks/eval/generate_synthetic_sf.py lines 460-461.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 24e66d64-d5a2-4493-a25b-a8ae27db2dff

📥 Commits

Reviewing files that changed from the base of the PR and between 063f519 and 3298c8a.

📒 Files selected for processing (4)

• src/sampleworks/eval/generate_synthetic_density.py
• src/sampleworks/eval/generate_synthetic_sf.py
• src/sampleworks/eval/synthetic_utils.py
• tests/eval/test_synthetic_utils.py

[DorisMai]

The fix seems to be enforcing n_jobs = 1 on CUDA device. Unless we plan on parallelize across multiple GPUs (which I don't think we need for the 40 synthetic dataset at the moment?), this PR looks good to me. Make sure to rebase before merge.