Adopt upstream sizing logic from ComputeManager - #3
Open
dotsdl wants to merge 4 commits into
Open
Conversation
Drops the local _compute_jobs_to_create method and the inline sizing logic in ScriptTemplateHPCManager.create_compute_services in favor of the upstream version on alchemiscale.compute.manager.ComputeManager added in OpenFreeEnergy/alchemiscale#502. Same formula (min(num_tasks, max_submit_per_cycle, remaining_capacity) // claim_limit with floor-to-1), one source of truth. Changes ------- * ScriptTemplateHPCManager.create_compute_services signature now (data, target) -> int. The body shrinks substantially: the sizing computation is gone, only the backend-specific health checks (check_job_health, verify_running_jobs, clear_successful_jobs) and the jobs_pending() gate remain. * Drop ScriptTemplateHPCManager._compute_jobs_to_create. The base class on the upstream now provides this concrete method. * Drop HPCManagerSettings.max_submit_per_cycle. Now inherited from upstream ComputeManagerSettings, which adds the field with the same default of 1. Tests ----- * Drop the eight test_sizing_* unit tests; the equivalent suite lives upstream in alchemiscale/tests/unit/compute/test_manager.py. * Replace test_create_compute_services_respects_sizing with the simpler test_create_compute_services_honors_target, which confirms only that given target=N we submit exactly N scripts. The arithmetic is upstream's responsibility now. * Adapt remaining create_compute_services tests to the new signature (passing target= explicitly). Test count: 54 -> 46 (matching the work that's now done upstream); all pass against the alchemiscale#502 branch. Depends on ---------- * OpenFreeEnergy/alchemiscale#502 (must merge first; this PR will not pass tests until the upstream changes are available). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dotsdl
marked this pull request as ready for review
June 8, 2026 21:16
Adds a conda test env and a CI workflow: builds the env, overlays alchemiscale from git main (until #502 is on a conda-forge release), installs the package, and runs the test suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
List alchemiscale's runtime deps explicitly in the test env, and restrict setuptools find to alchemiscale_hpc* so the new devtools/ dir doesn't trip flat-layout auto-discovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the hand-listed deps with the full OpenFreeEnergy/alchemiscale devtools/conda-envs/test.yml (main), plus pyyaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks
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
1 participant
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.
Draft until OpenFreeEnergy/alchemiscale#502 merges. This PR depends on the new
_compute_jobs_to_createmethod andmax_submit_per_cyclefield added there.Note
Targeting the
slurm-managerbranch (PR #2). Will rebase tomainonce #2 lands and #502 merges.Summary
Drops the local sizing logic from
ScriptTemplateHPCManagerin favor of the upstream version onalchemiscale.compute.manager.ComputeManager. Same formula (min(num_tasks, max_submit_per_cycle, remaining_capacity) // claim_limitwith floor-to-1), now centralized.Changes
ScriptTemplateHPCManager.create_compute_servicessignature:(data) -> int→(data, target) -> int. The body shrinks substantially — the sizing computation is gone, only the backend-specific health checks (check_job_health,verify_running_jobs,clear_successful_jobs) and thejobs_pending()gate remain.Drop
ScriptTemplateHPCManager._compute_jobs_to_create. The base class on the upstream now provides this concrete method.Drop
HPCManagerSettings.max_submit_per_cycle. Now inherited from upstreamComputeManagerSettings, which adds the field with the same default of 1. Existing user configs continue to work unchanged.Tests
test_sizing_*unit tests; the equivalent suite lives upstream inalchemiscale/tests/unit/compute/test_manager.py.test_create_compute_services_respects_sizingwith the simplertest_create_compute_services_honors_target, which confirms only that giventarget=Nwe submit exactly N scripts. Arithmetic is upstream's responsibility now.create_compute_servicestests to the new signature (passingtarget=explicitly).Test count: 54 → 46 (matching the work that's now done upstream); all pass against the
alchemiscale#502branch.Test plan
mainpytest tests/— 46 tests pass🤖 Generated with Claude Code