Backfill DSD using segfrac by region and period, not just for demands missing altogether

[idelder]

Response to issue #268 that the rabbit has been quitely squeeling about in out of diff range comments.

Previously, segfrac was used to fill in demands without a DSD but only if no DSD was defined for that demand commodity in ANY region or period. If DSD were defined for some region-period combinations but not others, the user would get an unhelpful error. Now, this fill-in is done for each missing region-period combo.

Why someone might define a DSD for some regions/periods and not others is not immediately clear but that seems like perfectly legitimate behaviour. This might create situations where DSDs are silently filled in with flat profiles because a user just forgot to add it for some region or period, but I suspect that's what we all assumed was already the behaviour anyway.

Also just generally cleaned up some tech debt in the create_demands function.

Summary by CodeRabbit

• Bug Fixes

  • More accurate filling of demand distributions when entries are entirely missing for specific region/period/demand groups.
  • Better detection and clearer errors when duplicate entries exist for the same time slice.
  • Missing time-slice gaps are now identified and logged as informational messages.

• Refactor

  • Reorganized demand-distribution handling to validate and fill data per region/period/demand group, improving partial-data behavior and robustness.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 73da4630-2b50-46db-a4a1-b6baf41af7f4

📥 Commits

Reviewing files that changed from the base of the PR and between dade39b and e9f34ca.

📒 Files selected for processing (1)

• temoa/components/commodities.py

---

Walkthrough

Refactors demand-distribution completion in create_demands() to perform per-(region, period, demand) checks using a precomputed grouping of existing distribution keys, and tightens validation and missing-slice computation accordingly.

Changes

Cohort / File(s)

Summary

Demand distribution logic temoa/components/commodities.py

Removed unused imports (sys, operator.itemgetter). Replaced blanket fill of missing DSD entries with per-(region, period, demand) filling when no keys exist for that group using segment_fraction[s,d]. Added dsd_keys_by_rpd grouping to drive validation and filling. Compute missing slices as all_time_slices - defined_slices for partially defined groups and log as info. Enforce max-one-key-per-time-slice per (r,p,dem) (raise ValueError if violated) and apply "sum to 1" checks to the actual keys present (or after auto-fill).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~35 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly summarizes the main change: shifting DSD backfilling logic from a demand-level approach to a region-and-period-specific approach using segfrac, which is the primary technical change in the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@temoa/components/capacity.py`:
- Around line 337-339: The KeyError occurs because v_retired_capacity is only
defined for v < p but the retirement_periods set can include v == p via
is_survival_curve_process; update the accesses to v_retired_capacity in the
annual retirement logic (the lines referencing v_retired_capacity[r, p_next, t,
v] and v_retired_capacity[r, p, t, v]) to first check v < p (and v < p_next as
appropriate) before reading/adding/subtracting those entries—i.e., wrap those
reads in a guard like "if v < p:" (and "if v < p_next:" for the p_next case) so
survival-curve (v == p) entries do not attempt to access undefined keys; keep
the rest of the branch logic unchanged.

In `@temoa/components/commodities.py`:
- Around line 745-754: The code currently logs missing demand-specific
distribution time slices with logger.info which hides important data; change the
logger call in the block that checks if len(keys) != expected_key_length to
logger.warning (keep the same message and variables: keys, expected_key_length,
all_time_slices, missing and the tuple (r, p, dem)) so partial DSD gaps are
surfaced at WARNING level (i.e., replace logger.info(...) with
logger.warning(...)).
- Around line 732-735: The loop in initialize_demands repeatedly calls
demand_specific_distribution.sparse_keys() and rebuilds the local list `keys`
for each (r,p,dem); instead, pre-index sparse_keys() once before iterating over
used_rp_dems into a lookup structure (e.g., a dict or set keyed by (r,p,dem)
mapping to the associated (_s,_d)/entries) and then use that lookup in the loop
and in the later membership checks (the places that currently build `keys` and
the membership test around the original lines 734 and 749); update references to
`keys` to pull from the pre-built index to avoid quadratic rescanning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 076c7b36-10bb-4fe3-b2f9-9b54f7d4993a

📥 Commits

Reviewing files that changed from the base of the PR and between 50c1346 and 2bbac58.

📒 Files selected for processing (8)

• temoa/components/capacity.py
• temoa/components/commodities.py
• temoa/components/utils.py
• temoa/data_io/component_manifest.py
• temoa/data_io/hybrid_loader.py
• temoa/model_checking/commodity_network_manager.py
• temoa/types/solver_types.py
• temoa/types/validation_types.py

[coderabbitai]

♻️ Duplicate comments (1)

temoa/components/commodities.py (1)

759-763: ⚠️ Potential issue | 🟡 Minor

Raise partial DSD gaps at warning level.

These omissions are not just informational: downstream code in temoa/components/capacity.py:437-444, temoa/components/reserves.py:314-320, temoa/components/emissions.py:107-114, and temoa/_internal/table_data_puller.py:189-196 reads demand_specific_distribution[r, p, s, d, ...] directly for each time slice, so missing entries materially become zero shares. Keeping this at info hides the only signal under the default WARNING log level.

♻️ Proposed fix

-            logger.info(
+            logger.warning(
                 'Missing some time slices for Demand Specific Distribution %s: %s',
                 (r, p, dem),
                 missing,
             )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@temoa/components/commodities.py` around lines 759 - 763, The log about
missing time slices in temoa/components/commodities.py currently calls
logger.info and should be raised to warning so downstream consumers (e.g.,
demand_specific_distribution lookups in capacity.py, reserves.py, emissions.py,
table_data_puller.py) see the issue by default; replace the logger.info(...)
call that logs 'Missing some time slices for Demand Specific Distribution %s:
%s' (currently passing (r, p, dem) and missing) with logger.warning(...)
preserving the message text and arguments so the exact same context is emitted
at WARNING level.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@temoa/components/commodities.py`:
- Around line 759-763: The log about missing time slices in
temoa/components/commodities.py currently calls logger.info and should be raised
to warning so downstream consumers (e.g., demand_specific_distribution lookups
in capacity.py, reserves.py, emissions.py, table_data_puller.py) see the issue
by default; replace the logger.info(...) call that logs 'Missing some time
slices for Demand Specific Distribution %s: %s' (currently passing (r, p, dem)
and missing) with logger.warning(...) preserving the message text and arguments
so the exact same context is emitted at WARNING level.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f01060a9-8894-46d3-9d23-86e07a845c97

📥 Commits

Reviewing files that changed from the base of the PR and between 2bbac58 and dade39b.

📒 Files selected for processing (1)

• temoa/components/commodities.py