You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add stratified periodic value-change counters for entity properties
This PR replaces the earlier incidence-oriented design with a more general value change counter architecture.
Instead of a single optional counter per property, each tracked property can now own many counters, each identified by a counter_id and configured with a strata type (PL: PropertyList<E>). This supports multiple independent reporting workflows (e.g. daily incidence, weekly incidence, ...) over the same property.
This replaces incidence_counts. Counter creation pushes a new trait object into this vector and returns its index as counter_id.
Counter trait and concrete implementation
The ValueChangeCounter trait erases the strata type. Concrete type: StratifiedValueChangeCounter<E, PL, P>
Backed by HashMap<(PL, P), usize>
On update, computes stratum PL for the entity and increments (PL, P) bucket
Exposes typed methods for reading and clearing counts
Where counts are updated
Counter updates occur in PartialPropertyChangeEventCore<E, P>::emit_in_context only on true transitions (current != previous).
Periodic behavior and phase semantics
period must be finite and > 0.
First report is at now + period.
Event emission is scheduled with add_plan_with_phase(..., ExecutionPhase::Last) so that any same-time plans/callbacks that might mutate counts run first.
After handler runs the counter is cleared / reset.
PropertyList changes
The old incidence-specific PropertyList helpers were removed.
PropertyList<E> now contributes one new strata-oriented helper:
get_values_for_entity(context, entity_id) -> Self
Used by StratifiedValueChangeCounter to compute PL during update(...).
Typical usage
let counter_id = context.create_value_change_counter::<Person,(InfectionStatus,),Age>();
context.track_periodic_value_change_counts::<Person,(InfectionStatus,),Age>(1.0,
counter_id,
|_context, counter| {// read counts by (stratum, new_value)let _new_age_count = counter.get_count((InfectionStatus::Infected,),Age(21));// write reports here},);
Test coverage in this PR
Counter IDs increment in insertion order.
Counters increment only on true transitions (no-op writes ignored).
Periodic handlers read expected counts and matched counters are cleared afterward.
Existing entity/query/index/event behavior remains green under full test suite.
Questions and Issues
Advanced use cases
The "low level" ContextEntitiesExt::create_value_change_counter method is public to support an advanced use case in which client code would want to wire up their own schedulers and handlers. But this is arguably a YAGNI violation. We could:
make create_value_change_counter private
eliminate the PeriodicValueChangeCountEvent entirely and just run the handler directly in the plan that in the current implementation emits this event
Resolved: This feels over-engineered. Let's support an advanced use case when we have one. Simplify the code for now.
Why the extra trait bounds (PL: Eq + Hash, P: Eq + Hash)
StratifiedValueChangeCounter<E, PL, P> stores counts in HashMap<(PL, P), usize>, which requires both PL and P to implement Eq + Hash. The bounds are intentionally local to the value-change-counter APIs, so we do not need to impose Eq/Hash on all Property implementors globally. #782 / #783 addresses adding Hash to the Property trait.
Start time
Resolved:
The start of recording of value changes ("creation of the index") happens at ExecutionPhase::First at simulation start time (default time=0, but settable via Context::set_start_time).
Recording value changes outside of simulation time is unsupported.
Start logic: First event handler for periodic value change count event fires at ExecutionPhase::Last at start time.
Since the handler fires in ExecutionPhase::Last, it's possible for plans & callbacks scheduled at start time to have changed property values, with these changes recorded in the value change counter. Thus, the counter can be nonempty when first handler event fires.
If client code starts simulation at negative time and wants handler to run at multiples of period, client code needs to set start time to a multiple of period.
Handler event fires at ExecutionPhase::Last at every $start\_time + k*period$, $k\in \mathbb{N}$ until simulation end.
End logic: If no plan is scheduled, the next handler event is not scheduled.
If no plan is scheduled, then the current plan is the last plan, and the simulation is about to end—there is nothing left to record.
The last period might be a "partial period." Client code will just need to understand this.
The handler's parameters
The periodic event handler is given
&Context: an immutable reference to the Context. A mutable reference is not required for reporting.
&mut StratifiedValueChangeCounter<E, PL, P>: a mutable reference to the counter. Since the counter is cleared when the handler returns, it doesn't matter if the handler mutates the counter.
We could do some work to try to give the handler a mutable reference to context. This might be worth spending a bit more time on.
The reason will be displayed to describe this comment to others. Learn more.
Based on our discussion, I would make the following changes:
Let's have an explicit start (implying we start it from the First phase) and panic if the current time is in the past (including if it's current time but during Normal)
I think the use of the tuple for multiple output types is confusing as it is a very common use case to want to have counts stratified by multiple properties (e.g., infection status and age group). I would instead give people a pattern for a shared function, and also have the docs show a multi property example if that's the way people need to count stratified properties
It's important that we support multiple periods for the same index sets. Could you maybe store them by period/start time and simply reuse instead of create on creation?
I would favour a more generic name personally (like aggregate) but I don't have a great alternative in mind, maybe somebody else does? If not we can leave it for now
For stratification, I think what we really want is this: When the tracked property value changes, e.g. Hospitalized, then we store whatever "stratification property values" the entity has at that very instant. This separates the properties we are using to stratify from the property whose values are changing.
The problem with using multi-properties is this: Suppose we count the multi-property (Age(31), Hospitalized(true)) during the period because a 31 year old enters the hospital. Now suppose by coincidence this person has a birthday before the period ends. The multi-property changes to the value (Age(32), Hospitalized(true)), so the system would count that value even though it's not a new hospitalization.
So we want to only count on changes of one property but record stratification property values at the instant of the change.
Multi-properties work for stratification only if the property values you want to use for stratification don't change.
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
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.
Updated
Add stratified periodic value-change counters for entity properties
This PR replaces the earlier incidence-oriented design with a more general value change counter architecture.
Instead of a single optional counter per property, each tracked property can now own many counters, each identified by a
counter_idand configured with a strata type (PL: PropertyList<E>). This supports multiple independent reporting workflows (e.g. daily incidence, weekly incidence, ...) over the same property.Public API
New
ContextEntitiesExtmethodscreate_value_change_counter<E, PL, P>(&mut self) -> usizeP, stratified byPL.track_periodic_value_change_counts<E, PL, P>(&mut self, period: f64, counter_id: usize, handler: ...)PeriodicValueChangeCountEvent<E, PL, P>.counter_id, calls handler on matching counter, then clears that counter.Internal design
Counter storage
PropertyValueStoreCore<E, P>now stores:value_change_counters: Vec<RefCell<Box<dyn ValueChangeCounter<E, P>>>>This replaces
incidence_counts. Counter creation pushes a new trait object into this vector and returns its index ascounter_id.Counter trait and concrete implementation
The
ValueChangeCountertrait erases the strata type. Concrete type:StratifiedValueChangeCounter<E, PL, P>HashMap<(PL, P), usize>PLfor the entity and increments(PL, P)bucketWhere counts are updated
Counter updates occur in
PartialPropertyChangeEventCore<E, P>::emit_in_contextonly on true transitions (current != previous).Periodic behavior and phase semantics
periodmust be finite and > 0.now + period.add_plan_with_phase(..., ExecutionPhase::Last)so that any same-time plans/callbacks that might mutate counts run first.PropertyList changes
The old incidence-specific
PropertyListhelpers were removed.PropertyList<E>now contributes one new strata-oriented helper:get_values_for_entity(context, entity_id) -> SelfUsed by
StratifiedValueChangeCounterto computePLduringupdate(...).Typical usage
Test coverage in this PR
Questions and Issues
Advanced use cases
The "low level"
ContextEntitiesExt::create_value_change_countermethod is public to support an advanced use case in which client code would want to wire up their own schedulers and handlers. But this is arguably a YAGNI violation. We could:create_value_change_counterprivatePeriodicValueChangeCountEvententirely and just run the handler directly in the plan that in the current implementation emits this eventResolved: This feels over-engineered. Let's support an advanced use case when we have one. Simplify the code for now.
Why the extra trait bounds (
PL: Eq + Hash,P: Eq + Hash)StratifiedValueChangeCounter<E, PL, P>stores counts inHashMap<(PL, P), usize>, which requires bothPLandPto implementEq + Hash. The bounds are intentionally local to the value-change-counter APIs, so we do not need to imposeEq/Hashon allPropertyimplementors globally. #782 / #783 addresses addingHashto thePropertytrait.Start time
Resolved:
ExecutionPhase::Firstat simulation start time (default time=0, but settable viaContext::set_start_time).ExecutionPhase::Lastat start time.ExecutionPhase::Last, it's possible for plans & callbacks scheduled at start time to have changed property values, with these changes recorded in the value change counter. Thus, the counter can be nonempty when first handler event fires.period, client code needs to set start time to a multiple ofperiod.ExecutionPhase::Lastat everyThe handler's parameters
The periodic event handler is given
&Context: an immutable reference to theContext. A mutable reference is not required for reporting.&mut StratifiedValueChangeCounter<E, PL, P>: a mutable reference to the counter. Since the counter is cleared when the handler returns, it doesn't matter if the handler mutates the counter.We could do some work to try to give the handler a mutable reference to
context. This might be worth spending a bit more time on.