Streamline system initialization refactor #1629
Draft
+560
−159
Conversation
This file contains 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
added 5 commits
October 29, 2024 18:01
Adds "Instant" counterparts to Clock and VariableClock, which report absolute times as Java instants, but still step time according to an Aerie duration. Also adds some minimal interoperability between absolute and relative clocks, and some useful derivations including comparisons. Using these, also adds a global Instant-based clock to Resources, along with exposing both the duration- and instant-based clocks as resources. In doing so, we add a new parameter for the plan's start time to Resources.init(). This in turn required refactoring some unit tests, and I took the opportunity to clean up the test construction a little bit as well. This revealed a way to correctly initialize Resources, i.e., to call Resources.init() exactly once per initialization of each test model. As a result, I refactored the clock handling to remove the awkward reinitialization pattern, since that pattern was added to handle test code.
Refactors streamline's init process, centralizing singletons and initialization into StreamlineSystem.
Using the absolute-timed InstantClock, add a new global simulation clock created by StreamlineSystem.init. Also, refactor the arguments to StreamlineSystem.init into an InitArgs object with an accompanying builder. This lets us write a "testBuilder" constructor that fills in as many init args with defaults as it can, while letting us override parameters if needed. This pattern will extend well if we need to add more parameters in the future. Using the global simulation clock, removes the "plan start time" parameter from the overload of DiscreteResources.precomputed that uses Instant keys. This guarantees that absolute-timed precomputed resources are synced with the global clock. This also creates a name collision with the version of precomputed that uses Duration keys, so the one with Instant keys was renamed to precomputed$.
|
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.
DRAFT pending #1589 , on which this PR builds.
Description
Refactors the initialization of streamline to an explicit call to StreamlineSystem.init. This new method is responsible for building all the singletons used by streamline, rather than putting that responsibility on the Registrar. This promotes a better separation of concerns, especially as we add more singleton-type functionality to initialize.
This also standardizes re-initialization behavior. Re-initializing an already-initialized system will simply rebuild all the singletons. While this would be wasteful if abused in production, it supports testing scenarios where multiple models may be built and re-built as tests are run.
Finally, after merging in #1589, this adds a globally-visible absolute clock, and relatedly the
StreamlineSystem.currentInstant()method to get the current time as an Instant.Verification
Existing unit tests were updated to call StreamlineSystem.init, and they continue to function properly.
The streamline-demo model was also updated to call this method, and was tested manually in a local aerie deployment. It continues to function correctly as well.
Documentation
IMPORTANT: This is a breaking change!
All projects that use streamline should change
into
when constructing their model. This will properly initialize the full streamline system.
Additionally,
currentTime()has moved fromResourcestoStreamlineSystem.Finally, while not a breaking change, this does expose
StreamlineSystem.currentInstant(), which returns the current absolute time as an Instant. This is something I expect many might find useful.Future work
N/A