Feature/simulator worker integration#35
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves simulator-worker integration by aligning time series field-name recognition with simulator outputs, adding a string-in/string-out ESDL KPI embedding API, and updating user docs accordingly.
Changes:
- Recognize simulator-produced time series columns (
heat_demand,heat_supplied) so energy/emission KPIs are computed from simulator results. - Add
KpiManager.build_esdl_string_with_kpis()to embed KPIs into an existing ESDL XML string. - Update documentation examples/references for simulator and in-memory workflows; adjust dependency groups (runtime vs lint/type stubs).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates locked dependencies to reflect dependency/group changes (removals and moved stubs/tools). |
src/kpicalculator/kpi_manager.py |
Adds build_esdl_string_with_kpis() for embedding KPIs into an ESDL XML string. |
src/kpicalculator/common/constants.py |
Extends recognized time series field-name tuples to include simulator column names. |
pyproject.toml |
Moves typing stubs to lint deps; removes unused runtime dependency and updates mypy overrides. |
doc/user_documentation/kpi_reference.rst |
Documents additional recognized simulator field names for demand/production. |
doc/getting_started.rst |
Updates API reference and adds simulator/in-memory workflow guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jarsarasty
force-pushed
the
feature/simulator-worker-integration
branch
from
29805c2 to
f3d4ece
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves OMOTES simulator-worker integration by aligning supported time-series field names with simulator outputs, adding a string-in/string-out KPI embedding API, and updating docs/tests accordingly.
Changes:
- Recognize simulator column names (
heat_demand,heat_supplied) in the KPI calculators’ time-series field lookup lists so energy/emission KPIs don’t default to 0.0. - Add
KpiManager.build_esdl_string_with_kpis()to embed KPI results directly into an ESDL XML string and return the updated string. - Update unit tests and user documentation to cover the new API and simulator integration workflow; adjust dev/lint dependency grouping for type stubs.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/kpicalculator/common/constants.py |
Extends recognized demand/production field names to include simulator columns so calculators can find the right time series. |
src/kpicalculator/kpi_manager.py |
Adds build_esdl_string_with_kpis() for in-memory (string-based) KPI embedding into ESDL. |
unit_test/test_examples.py |
Adds coverage for the new string-in/string-out KPI embedding method and verifies manager state restoration. |
doc/user_documentation/kpi_reference.rst |
Documents updated time-series key lookup behavior including simulator-specific keys. |
doc/getting_started.rst |
Updates API reference and adds a simulator-worker integration example using the new embedding method. |
pyproject.toml |
Moves type stubs to dev/lint groups and removes unused/runtime-only dependencies from core dependencies. |
uv.lock |
Lockfile updates reflecting dependency cleanup and dev/lint dependency adjustments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jarsarasty
force-pushed
the
feature/simulator-worker-integration
branch
2 times, most recently
from
cc4b2dc to
4e24662
Compare
jarsarasty
force-pushed
the
feature/simulator-worker-integration
branch
2 times, most recently
from
f45a350 to
ed84224
Compare
This was referenced May 6, 2026
Closed
…is, docs
- Add heat_demand and heat_supplied to DEMAND_FIELDS and PRODUCTION_FIELDS
so energy/emission KPIs are non-zero when loaded from simulator results
- Add KpiManager.build_esdl_string_with_kpis() for string-in/string-out
KPI embedding into an existing ESDL XML string
- Add test for build_esdl_string_with_kpis with value round-trip assertions
- Fix cast(str, ...) to satisfy mypy no-any-return on esh.to_string()
- Fix non-runnable doc examples and add simulator integration section
- Update kpi_reference.rst: heat_demand/heat_supplied in time series key tables
…RATE_PERCENT from public API Callers (e.g. simulator-worker) that resolve absent config values to library defaults can now import these constants from `kpicalculator` directly instead of reaching into the private `kpicalculator.common.constants` module.
- Remove coloredlogs: unused in source code
- Move pandas-stubs and types-xmltodict from runtime to lint dev
dependencies: type stubs have no runtime use and caused version
conflicts downstream (e.g. pandas-stubs pin in simulator-worker)
- Remove radon and bandit from analysis group: both are covered by
ruff (C9 for complexity, S ruleset for security)
- Remove coloredlogs from mypy overrides
- Add calculate_eac() and calculate_tco() to CostCalculator
- Add discount_rate field to Asset model, extracted from ESDL costInformation.discountRate
- Add round_up_replacement parameter to calculate_npv(), calculate_lcoe(), and
calculate_tco() for optimizer-compatible continuous replacement factor;
threaded through calculate_all_kpis() and the public API
- Extend ESDL adapter to extract discount rate and maintenance costs
- Add EAC and TCO sections to kpi_guide.rst with formulas and usage guidance
- Add unit tests for EAC, TCO, NPV properties, and per-asset discount rate override
…sset financial breakdown
- Rename cost_calculator.py → financial_calculator.py; class CostCalculator → FinancialCalculator
- Add AssetFinancialResult TypedDict with 11 fields (investment, installation, fixed/variable
operational, fixed/variable maintenance, annualized_capex, eac, npv, tco, lcoe)
- Add FinancialCalculator.get_asset_financial_breakdown() — single-pass computation for all
asset-level and system-level financial KPIs
- Add FinancialCalculator.aggregate_by_category() — O(n) CAPEX/OPEX breakdown from asset dict;
replaces get_capex_by_category() / get_opex_by_category() (removed)
- Add FinancialCalculator._compute_discounted_sum() — eliminates 4x duplication of discounted
sum + fractional year pattern across calculate_npv(), calculate_lcoe(), and new helpers
- KpiResults["costs"] → KpiResults["financials"]; CostResults → FinancialResults
- KpiResults["asset_financials"] added — per-asset breakdown exposed to callers and exporters
- Per-asset LCOE computed inside get_asset_financial_breakdown() via annual_energy_mwh_by_asset
parameter; None for non-producers or zero-energy producers
- System totals (NPV, EAC, TCO) derived by summing asset values — single loop in kpi_manager
- Remove dead methods: get_capex/opex_by_category, _calculate_capex/opex_for_category,
_asset_belongs_to_category
- Rename PRODUCER_ASSET_TYPES (was _PRODUCER_ASSET_TYPES) — public constant used across modules
- Add AssetFinancialBreakdownTest (20 tests) and PerAssetLcoeTest (4 tests); 401 tests passing
- Update all docs: getting_started.rst (asset_financials in results structure), architecture.rst,
api_reference.rst, kpi_guide.rst, IMPLEMENTATION_SUMMARY.md, ROADMAP.md, DESIGN.md
…tion
- Add field-level docstrings to KpiResults, FinancialResults, EnergyResults,
EmissionResults, and AssetFinancialResult TypedDicts
- Export result types from the public package API and add them to api_reference.rst
- Fix per-asset discount rate: npv and lcoe now use asset-specific rates via
get_asset_financial_breakdown(); calculate_lcoe() no longer calls calculate_npv()
- Remove unused discount_rate_ratio parameter from _compute_asset_result()
- Add runtime warning when unsupported export level is passed
- Document geothermal COP adjustment, MESIDO TCO maintenance cost exclusion,
level parameter limitations, and calculate_npv() uniform-rate limitation
- Reduce redundancy between rst docs and docstrings; cross-reference API reference
from getting_started.rst and kpi_guide.rst
Add calculate_kpis_from_simulator() and build_esdl_string_with_kpis() as standalone functions so callers need no knowledge of KpiManager internals. EsdlKpiExporter.export() now takes esdl.EnergySystem directly, removing the coupling to the common model and the KpiManager state-swap workaround. Simulator worker updated to use the public API only; fixes a semantic bug where output_esdl (no cost data) was passed instead of input_esdl.
- Extract KPI name magic strings to module-level constants - Consolidate duplicate ESDL system factory into _create_mock_esdl_system() - Add _find_distribution_item() and _get_kpi_count() helpers to reduce duplication - Apply AAA (Arrange-Act-Assert) pattern throughout all tests - Replace bare assert with self.assertIsInstance() for consistent error reporting - Fix invalid msg parameter in assertRaises() context manager - Use TemporaryDirectory context manager instead of manual mkdtemp/tearDown - Fix invalid msg parameter in assertRaises() context manager
jarsarasty
force-pushed
the
feature/simulator-worker-integration
branch
from
e27b1f7 to
eb76d3f
Compare
jarsarasty
force-pushed
the
feature/simulator-worker-integration
branch
from
eb76d3f to
6abe4db
Compare
samvanderzwan
approved these changes
May 7, 2026
samvanderzwan
left a comment
samvanderzwan
left a comment
Contributor
There was a problem hiding this comment.
Small comment on documentation, for the rest looks good
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.
heat_demandandheat_suppliedto the energy/emission field lookup tuples so KPIs are non-zero when loading simulator results viaload_from_simulator(). Previously all energy and emission KPIs returned 0.0 because the simulator's column names did not match any recognised field name.KpiManager.build_esdl_string_with_kpis()— a string-in/string-out method that embeds KPI results into an existing ESDL XML string, providing a clean API for callers without requiring external manipulation of internal state.build_esdl_string_with_kpiswith value round-trip assertions and state preservation check.get_esdl_with_kpis→build_esdl_with_kpis).To be merged after #32, #33, #34