Add InstantClock and VariableInstantClock support #1589
Conversation
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.
| */ | ||
| public record InstantClock(Instant extract) implements Dynamics<Instant, InstantClock> { | ||
| @Override | ||
| public InstantClock step(Duration t) { |
There was a problem hiding this comment.
Just musing - would it make any sense for InstantClock to be derived from the Duration Clock? Or do you find that separating them is cleaner?
There was a problem hiding this comment.
Java won't let us derive one from the other, because they both implement Dynamics but with different type arguments. I also kind of like that an Instant clock doesn't necessarily have a preferred "zero" / "epoch" time, and you can just create a Duration clock by giving a zero time that makes sense in context.
|
|
||
| // TODO - this method belongs somewhere else... | ||
| // Making it package-private at least lets us move it later without dependency issues outside the library. | ||
| static Duration durationBetween(Instant start, Instant end) { |
There was a problem hiding this comment.
In a perfect world, we'd have the Duration class in merlin-sdk be the thinnest possible wrapper around an integer in microseconds, and the framework would define decorator methods like this one that would enhance its usability. As it stands... it probably makes the most sense to add this method to the Duration class itself.
| public static Duration currentTime() { | ||
| try { | ||
| return currentValue(CLOCK); | ||
| } catch (Scoped.EmptyDynamicCellException | IllegalArgumentException e) { |
There was a problem hiding this comment.
Nice to see this exception handler go away. Not entirely sure I follow what the paradigm shift was that made it obsolete
There was a problem hiding this comment.
In hindsight, it was obvious. I just had to put a call to Resources.init() (or build a streamline registrar) in the constructor for my unit tests, so the unit tests would re-init the system at the correct time. The unit test is just a model, so it's exactly the same paradigm that we use in production. I don't know why I didn't realize I could do that before. I think maybe I was originally trying to avoid the need for an init method altogether?
Description
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.
Verification
Simple tests added for simulationClock() and absoluteClock() added to Resources, which are likely to be the most-used parts of this PR. I'll test other types and functions in this PR incidentally as I prototype some other ideas, and update this PR with those results..
Documentation
Ideally these should be mentioned in the streamline users' guide.
Future work
Although absolute clocks are often useful in their own right, this is specifically in support of command expansion and incon/fincon prototypes I'm working on.