Emod api 3.0 to support emodpy-malaria 2.0 conversion #83
Open
stitova-idm wants to merge 6 commits into
Open
Conversation
… and Coordiantor- level events.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates emod-api 3.0 to support emodpy-malaria 2.0 conversion by removing migration-file tooling (moving it out of this repo), expanding demographics to support NodeProperties / NodePropertyValues, and redesigning campaign event tracking to support individual/node/coordinator event registration and validation.
Changes:
- Removed
emod_api.migrationimplementation and its tests/data (breaking change). - Added NodeProperties support in demographics (new
NodeProperty/NodePropertiesclasses, load/save, and per-node overrides viaNodePropertyValues). - Reworked campaign module event tracking and schema-driven built-in event discovery; added
BirthRateDependenceenum and expandedset_birth_rate()behavior/validation.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/test_node_properties.py | Adds unit tests for new NodeProperty / NodeProperties classes. |
| tests/unittests/test_migration_imports.py | Removes tests that asserted the old migration API surface. |
| tests/test_node.py | Adds tests for NodeAttributes.node_property_values serialization/roundtrip. |
| tests/test_migration.py | Removes migration functionality tests (module deleted). |
| tests/test_demographics.py | Updates tests for removed malaria-specific attributes and adds NodeProperties + expanded birth-rate tests. |
| tests/test_campaign_module.py | Replaces legacy campaign tests with schema-aware + event-registration/validation tests. |
| tests/data/demographics/single_node_demographics.json | Updates fixture to remove malaria-only keys and adjust ordering/values to match new serialization. |
| tests/data/demographics/Namawala_four_node_demographics_for_Thomas.json | Removes malaria-only risk keys and updates fixture formatting/ordering. |
| tests/data/demographics/demographics_test_from_file_sets_necessary_simple_distribution_implicit_functions.json | Removes malaria-only keys from implicit-function fixture. |
| tests/data/demographics/demographics_node_properties.json | Adds a reference fixture for NodeProperties + NodePropertyValues roundtrip. |
| emod_api/utils/str_enum.py | Adds __repr__ returning .value for string enums. |
| emod_api/utils/emod_enum.py | Introduces BirthRateDependence enum (used by set_birth_rate). |
| emod_api/migration/README.md | Deletes migration module documentation (module removed). |
| emod_api/migration/migration.py | Deletes migration implementation (module removed). |
| emod_api/migration/main.py | Deletes migration CLI entrypoint (module removed). |
| emod_api/demographics/properties_and_attributes.py | Adds NodeProperty / NodeProperties; removes malaria-only IndividualAttributes fields; adds NodeAttributes.node_property_values. |
| emod_api/demographics/node.py | Removes malaria-only private helper methods from Node. |
| emod_api/demographics/implicit_functions.py | Reworks birth-rate dependence implicits; removes demog risk implicit. |
| emod_api/demographics/demographics.py | Loads NodeProperties from JSON in from_file(). |
| emod_api/demographics/demographics_base.py | Adds top-level NodeProperties serialization and expands set_birth_rate() to support multiple dependence modes. |
| emod_api/campaign.py | Replaces legacy ad-hoc event mapping with explicit individual/node/coordinator event tracking + validation and schema-derived built-ins. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
30
to
+48
| def reset(): | ||
| """Reset all campaign state to defaults. | ||
|
|
||
| Clears accumulated events, signal tracking lists, event mappings, | ||
| and the schema cache. | ||
| """ | ||
| campaign_dict["Events"].clear() | ||
|
|
||
| pubsub_signals_subbing.clear() | ||
| pubsub_signals_pubbing.clear() | ||
| adhocs.clear() | ||
| custom_coordinator_events.clear() | ||
| custom_node_events.clear() | ||
| individual_events_listened.clear() | ||
| individual_events_broadcast.clear() | ||
| node_events_broadcast.clear() | ||
| node_events_listened.clear() | ||
| coordinator_events_broadcast.clear() | ||
| coordinator_events_listened.clear() | ||
| implicits.clear() | ||
| individual_builtin_events.clear() | ||
| node_builtin_events.clear() | ||
| coordinator_builtin_events.clear() | ||
| s2c.clear_schema_cache() |
Comment on lines
+212
to
+218
| broadcast_not_listened = broadcast - listened | ||
| if broadcast_not_listened: | ||
| warnings.warn( | ||
| f"The following {level} events are broadcast but nothing is listening to them within " | ||
| f"the campaign: {sorted(broadcast_not_listened)}") | ||
|
|
||
| return list(broadcast) |
Comment on lines
+232
to
+240
| super().__init__() | ||
| if initial_distribution: | ||
| for i in initial_distribution: | ||
| if i < 0 or i > 1: | ||
| raise ValueError("initial_distribution values must be between 0 and 1.") | ||
| if sum(initial_distribution) != 1: | ||
| raise ValueError("initial_distribution values must sum to 1.") | ||
| if len(initial_distribution) != len(values): | ||
| raise ValueError("initial_distribution must have the same number of entries as values.") |
Comment on lines
+246
to
+251
| def to_dict(self) -> dict: | ||
| node_property = self.parameter_dict | ||
| node_property.update({"Property": self.property, "Values": self.values}) | ||
| if self.initial_distribution: | ||
| node_property.update({"Initial_Distribution": self.initial_distribution}) | ||
| return node_property |
Comment on lines
+288
to
+294
| birth_rate_dependence: How EMOD uses the BirthRate value. | ||
| Accepts a :class:`~emod_api.demographics.implicit_functions.BirthRateDependence` | ||
| member or its string value. Defaults to ``POPULATION_DEP_RATE``. | ||
| - ``FIXED_BIRTH_RATE`` — 'rate' is used as an absolute daily birth rate with which new individuals are born. | ||
| units: number of births per year | ||
| - ``POPULATION_DEP_RATE`` — 'rate' is scaled by node population to determine the daily birth rate. | ||
| units: number of births per 1000 people per year |
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.
Summary of changes: support_node_coordinator_events vs main
4 commits, 21 files changed, 1,420 additions, 2,226 deletions
Commits:
55c0875 adding node properties, removing malaria-specific demographics elements
a8e38d1 adding missing tests
848b513 aligning with main
ac64da3 first checkin; remove migration (it will be in emodpy), adding Node- and Coordinator- level events.
BREAKING CHANGES
The entire emod_api/migration/ package (782-line migration.py, init.py,
main.py, README.md) has been deleted. All migration functionality (binary
file reading/writing, CSV export, MigrationFile class, Migration class,
to_csv(), examine_file()) is gone. Per the commit message, migration will move
to emodpy.
Impact: Any code doing
import emod_api.migrationorfrom emod_api.migration.migration import ...will break.Module-level variables and functions in emod_api/campaign.py have been renamed:
Old name New name
pubsub_signals_subbing individual_events_listened
pubsub_signals_pubbing individual_events_broadcast
adhocs removed
custom_coordinator_events coordinator_events_broadcast
custom_node_events node_events_broadcast
event_map removed
trigger_list removed
Impact: Any code referencing campaign.pubsub_signals_subbing,
campaign.event_map, campaign.adhocs, etc. will break.
nameparameter removed, replaced withnoteparameterfirstparameter removed entirely (was already deprecated)Old: campaign.add(event, name=None, first=False)
New: campaign.add(event, note: str = None)
Impact: Code using campaign.add(event, name="X") will fail (keyword arg
renamed). Code using campaign.add(event, first=True) will fail.
These functions no longer exist. get_event() used to manage the GP_EVENT_xxx
mapping for ad-hoc events. Events are now passed through as-is -- no more
GP_EVENT remapping.
Impact: Any code calling campaign.get_event() or campaign.get_trigger_list()
will break.
Was returning event_map. No replacement -- the ad-hoc event mapping concept
is gone.
These constructor parameters and properties were removed from
emod_api.demographics.properties_and_attributes.IndividualAttributes:
innate_immune_distribution2
And the corresponding RiskDistribution* / InnateImmuneDistribution*
serialization/deserialization code.
Impact: Any code setting these attributes directly will fail. This is
intentional -- these belong in emodpy-malaria's MalariaDemographics subclass.
Removed from emod_api.demographics.node.Node:
Impact: These were private (_-prefixed) so external callers should be rare,
but emodpy-malaria was likely using them.
emod_api.demographics.implicit_functions._set_enable_demog_risk (which set
Enable_Demographics_Risk = 1) is deleted. Risk distribution config handling
must now be done in the disease-specific package.
Old: set_birth_rate(self, rate: float, node_ids: list[int] = None)
New: set_birth_rate(self, rate: float, node_ids: list[int] = None,
birth_rate_dependence: Union[str, BirthRateDependence] = "POPULATION_DEP_RATE")
The new parameter is additive (has a default), so existing calls won't break
syntactically. However, the unit interpretation changed -- the function now
validates rate ranges per dependence mode and converts units accordingly,
which could produce different runtime behavior for code relying on the old
fixed conversion (rate / 365 / 1000).
NEW FEATURES (non-breaking)
New module-level lists and functions in campaign.py:
existing coordinator/node getters)
set_schema() now auto-extracts built-in event lists from ReportEventRecorder,
ReportEventRecorderNode, and ReportEventRecorderCoordinator entries in the
schema. Stored in individual_builtin_events, node_builtin_events,
coordinator_builtin_events.
New validation logic raises ValueError if events are listened to but never
broadcast. Warns if events are broadcast but nothing listens. Built-in events
are excluded from validation.
New classes in properties_and_attributes.py for EMOD's top-level
NodeProperties -- property keys, values, and initial distributions on nodes.
New methods for adding node properties and setting per-node overrides via
NodePropertyValues in NodeAttributes.
New optional attribute for per-node property value overrides.
Reads NodeProperties from JSON when loading demographics files.
New enum in emod_api.utils.emod_enum with values FIXED_BIRTH_RATE,
POPULATION_DEP_RATE, DEMOGRAPHIC_DEP_RATE, INDIVIDUAL_PREGNANCIES. Also adds
emod_api.utils.str_enum.StrEnum base class.
Validates rate ranges and converts units based on mode. Supports
INDIVIDUAL_PREGNANCIES for pregnancy-based targeting in campaigns.