Skip to content

Refactor reservation rule generation#347

Draft
tvk134 wants to merge 1 commit into
sequence-toolbox:masterfrom
tvk134:refactor-rule-generation-factory
Draft

Refactor reservation rule generation#347
tvk134 wants to merge 1 commit into
sequence-toolbox:masterfrom
tvk134:refactor-rule-generation-factory

Conversation

@tvk134

@tvk134 tvk134 commented May 15, 2026

Copy link
Copy Markdown

This PR extracts reservation-generated entanglement management rule construction from ResourceManager.generate_load_rules into a DefaultReservationRuleGenerator.

Previously, ResourceManager.generate_load_rules directly constructed the default entanglement generation, purification, and swapping rules before scheduling them. This refactor preserves the default rule construction behavior while allowing ResourceManager to delegate rule creation to a replaceable rule generator.

The ResourceManager still handles memory selection, reservation attachment, rule load/expiration scheduling, and memory reset scheduling. The new DefaultReservationRuleGenerator owns only the construction of reservation-created Rule objects.

This provides a cleaner extension point for experiments that need custom reservation rule generation without monkeypatching ResourceManager.

Tests added:

  • default generator creates expected rules for a direct path
  • ResourceManager calls a custom rule generator with the expected context
  • subclasses can override purification rule creation while inheriting default generation and swapping rules

Validation:

  • uv run ruff check tests/resource_management/test_rule_generation.py sequence/resource_management/rule_generation.py sequence/resource_management/resource_manager.py
  • uv run pytest tests

@hayekr

hayekr commented May 15, 2026

Copy link
Copy Markdown
Collaborator

HI @tvk134, thank you for the contribution

Do you plan on adding functionality to swap out individual rules in this new rule generator. I notice that the new rule generator class still imports the built-in rules directly.

Let's suppose a user wants to only swap out one rule, is the design choice here to require the user to write an entirely new RuleGenerator?

@hayekr hayekr added the feature New feature or request label May 15, 2026
@tvk134

tvk134 commented May 15, 2026

Copy link
Copy Markdown
Author

Hi @hayekr , thank you for the quick response!

That makes sense, and I agree that this should support individual rule replacement. I kept this first change minimal to separate rule construction from ResourceManager scheduling while preserving the existing behavior.

Right now the generator supports category-level overrides by subclassing create_generation_rules, create_purification_rules, or create_swapping_rules. It does not yet expose the individual built-in rules as swappable components, so I agree that the default generator still has some internal hardcoding through the direct imports.

I can revise this PR to split the default generator into smaller per-rule builder methods, e.g. create_eg_await_rule, create_eg_request_rule, create_ep_request_rule, create_ep_await_rule, create_es_a_rule, create_es_b_rule, etc. The default implementations would still use the existing built-in action/condition functions, preserving current behavior, but subclasses could override one rule at a time without copying an entire category method.

Would you prefer that approach, or would a named registry/mapping of rule builders be more consistent with the direction you have in mind?

@hayekr hayekr marked this pull request as draft May 15, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants