Feature/throughput reorg#56
Open
jaredraycoleman wants to merge 39 commits into
Open
Conversation
Both jobs were using Python 3.11 which doesn't meet the minimum requirement of >=3.12 specified in pyproject.toml. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…e to new method but can be changed back
…t works yet. Needs fixing
There was a problem hiding this comment.
Pull request overview
This PR reorganizes the online/throughput scheduling code by replacing the older observer/controller model with a step-driven Environment plus pluggable OnlinePolicy implementations, and updates tests/examples to match the new module layout.
Changes:
- Refactors the online simulation loop to
Environment(step_fn + policy.update)and introduces a newonline.policypackage (Inspirit, rescheduling, frontier fill). - Splits environment variants into
online/environment/{frontier,stochastic}.pyand updates online algorithms/tests/scripts imports accordingly. - Improves test determinism/output handling and updates wfcommons distribution fitting KS-test call; adds pytest
testpaths.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_online_step_strategies.py | Updates OnlineHEFT import path; seeds RNG in fixtures for determinism. |
| tests/test_inspirit_fifo.py | Migrates Inspirit tests to Environment + InspiritPolicy and tmp output path. |
| tests/test_fifo_scheduler.py | Renames/updates FIFO tests to FrontierFillPolicy and new module paths. |
| src/saga/schedulers/throughput/inspirit.py | Adapts throughput Inspirit scheduler to new policy-based environment; adds class docstring. |
| src/saga/schedulers/throughput/init.py | Introduces throughput package exports. |
| src/saga/schedulers/online/policy/reschedule.py | New ReschedulePolicy extracted from prior controller logic. |
| src/saga/schedulers/online/policy/inspirit.py | Refactors Inspirit controller into InspiritPolicy using policy/update semantics. |
| src/saga/schedulers/online/policy/frontier_fill.py | New FrontierFillPolicy extracted from prior frontier controller. |
| src/saga/schedulers/online/policy/_partial.py | New shared helper to build partial schedules from committed tasks. |
| src/saga/schedulers/online/policy/init.py | Defines OnlinePolicy interface and re-exports concrete policies. |
| src/saga/schedulers/online/environments.py | Removes old combined environments module in favor of the new package layout. |
| src/saga/schedulers/online/environment/stochastic.py | New StochasticEnvironment module using policy + step functions. |
| src/saga/schedulers/online/environment/frontier.py | New FrontierEnvironment module using policy + step functions. |
| src/saga/schedulers/online/environment/init.py | Refactors core Environment loop; introduces step functions and re-exports variants. |
| src/saga/schedulers/online/components.py | Removes old trigger/observer/controller abstractions superseded by policies. |
| src/saga/schedulers/online/algorithms/online_heft.py | Updates OnlineHEFT environment to use ReschedulePolicy. |
| src/saga/schedulers/online/algorithms/frontier_heft.py | Updates FrontierHEFT to use FrontierFillPolicy and new environment module. |
| src/saga/schedulers/online/algorithms/fifo.py | Updates FIFO/InspiritFIFO to use policy-based environment construction. |
| src/saga/schedulers/online/algorithms/init.py | Adds algorithms package exports for new module layout. |
| src/saga/schedulers/online/init.py | Re-exports environment/policy/algorithm APIs under saga.schedulers.online. |
| src/saga/schedulers/data/wfcommons.py | Adjusts KS-test invocation for fitted scipy distributions. |
| scripts/experiments/throughput_experiment/test_montage.py | Updates experiment script to new policy/step imports and algorithm paths. |
| scripts/experiments/throughput_experiment/test_inspirit.py | Updates experiment script to policy-based environment and new imports. |
| scripts/experiments/throughput_experiment/run_throughput.py | Updates experiment runner imports to new algorithm paths. |
| scripts/examples/test_refactor/main.py | Updates example to use policy-based environment and new imports. |
| scripts/examples/frontier_heft_vs_fifo/main.py | Updates example imports to new algorithm package paths. |
| scripts/examples/big_example_environment/main.py | Updates example to use InspiritPolicy and step functions. |
| scripts/examples/basic_example_environment/main.py | Updates example to use InspiritPolicy and step functions. |
| pyproject.toml | Adds pytest testpaths configuration. |
Comments suppressed due to low confidence (1)
src/saga/schedulers/online/policy/inspirit.py:294
- In the frontier path, this policy mutates
env.schedulein-place when it pinstask_name, but then returns whateverfill_policy.update(env)returns. If the fill policy returnsNone(e.g., frontier is empty after removing the pinned task),Environment.step()will skip the post-policy_update_task_state()/_update_network_state()refresh, leavingready_tasks/running_tasksand the history record stale for that step.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+9
| from saga.schedulers.throughput.inspirit import InspriritScheduler | ||
| from saga.schedulers.throughput.mt_scheduler import MTScheduler | ||
| from saga.schedulers.throughput.multi_obj import MultiObjScheduler | ||
|
|
||
| __all__ = [ | ||
| "InspriritScheduler", | ||
| "MTScheduler", | ||
| "MultiObjScheduler", | ||
| ] |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/saga/schedulers/online/policy/inspirit.py:131
- InspiritPolicy.reset() clears dispatch-threshold fields (dec_step/s_inc/s_dec and c) earlier in this method, but update() only calls _init_ranks() when efficiency_ranks is None. If the same policy instance is reused across multiple Environment.run() calls, subsequent runs will keep efficiency_ranks yet operate with zeroed thresholds, changing dispatch behavior. Preserve these configuration/default fields across reset() (or reinitialize them each run).
Comment on lines
+67
to
+71
| efficiency_ranking = [] | ||
| # for task in task_graph.tasks: | ||
| # ability = compute_task_inspiring_ability(task) | ||
| # # counter += 1 | ||
| # # heapq.heappush(effeciency_ranking, (-ability, counter, task.name)) | ||
| effeciency_ranks = {task.name: compute_task_inspiring_ability(task) for task in task_graph.tasks} | ||
| return effeciency_ranks | ||
| # # heapq.heappush(efficiency_ranking, (-ability, counter, task.name)) |
…tions, experiment rewrite - Placement constraints: node_constraints as first-class per-instance data on Schedule and StochasticSchedule, enforced in add_task, get_earliest_start_time, GreedyInsert, FastestNode, and threaded through the online Environment/StochasticEnvironment and policies; ConstraintViolation exception. - Data-structure optimizations: cached name/pair lookups for Network/TaskGraph and their stochastic counterparts (get_node/get_edge/get_task/in_edges/out_edges/get_dependency), O(1) is_scheduled, O(1) determinize successor lookup, cached mean-determinized graphs, deterministic (sorted) iteration to remove PYTHONHASHSEED dependence. - Experiment rewrite: consolidated runner (run.py), instance builders for the RIoTBench and WfCommons branches (instances.py), analysis/figures (analyze.py); removed the old ad hoc scripts; added a 300-task cap for WfCommons generation. - Tests for deterministic, stochastic, and online constraint handling.
Conflict resolution favored reorg's design where the two branches diverged,
since main is being merged into the active throughput-reorg feature branch.
Key decision: main and reorg made incompatible, competing refactors of the
scheduler core. Main converted Scheduler to a pydantic BaseModel with
Field-based attributes, made Schedule._task_map a List[ScheduledTask] per
name, and added task duplication (utils/duplication.py). Reorg instead keeps
Scheduler as a plain ABC with __init__-style schedulers, a single-valued
_task_map with _compute_load/_comm_load throughput aggregates, and its own
stochastic/parametric rewrite. These are two coherent but mutually exclusive
designs, so reorg's design was kept wholesale for every design-coupled file:
src/saga/__init__.py, schedulers/{cpop,fcp,gdl,heft,smt,wba}.py,
schedulers/hybrid.py, schedulers/parametric/{__init__,components}.py,
stochastic.py
Main's competing duplication support (utils/duplication.py) was dropped;
its own commit noted it was WIP and not yet working, and nothing in reorg
imports it.
Absorbed from main (independent of the design split): CI/publish workflows,
release.sh, README, pyproject, uv.lock (regenerated), pisa updates, example
scripts, and benign ruff/lambda reformatting in olb/flb/draw/random_graphs
and the wfcommons type-narrowing.
All 146 tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Brings in main's design of making Scheduler a pydantic BaseModel with Field-based configuration attributes instead of __init__/self-assignment, while keeping reorg's single-valued _task_map throughput design and its incremental schedule()/min_start_time API. Duplication was intentionally left out (it belongs on its own WIP branch). - __init__.py: Scheduler(ABC) -> Scheduler(ABC, BaseModel), keeping reorg's schedule(schedule, min_start_time) signature and the name property. - smt/fcp/gdl/wba: taken from origin/main (pure pydantic conversion; their schedule() bodies already matched reorg and already carried min_start_time). - hybrid, throughput/inspirit, online/algorithms/fifo (InspiritFIFOScheduler): converted __init__ config attrs to Field declarations. - throughput/multi_obj: removed per-call scratch state from self (network, task_graph, _rankings); rank map is now a local threaded into dominates(). - parametric/online (OnlineParametricScheduler): config as Fields; derived EstimateStochasticScheduler built in model_post_init as a PrivateAttr. Reorg-only Environment/Policy classes and StochasticScheduler-based schedulers are unaffected (they don't inherit Scheduler). All 146 tests pass; scheduler serialization via model_dump() now works; online and parametric examples run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
No description provided.