Skip to content

Conversation

@ParticularlyPythonicBS
Copy link
Member

@ParticularlyPythonicBS ParticularlyPythonicBS commented Jan 1, 2026

Summary by CodeRabbit

  • Documentation
    • Added a comprehensive database–model comparison guide mapping database tables to model sets and parameters across time, geography, technology, commodity, and parameter domains.
    • Integrated the new reference into the mathematical formulation documentation to improve clarity of model–database relationships and cross-references.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 1, 2026

Walkthrough

Adds a new documentation page mapping database tables to model sets/parameters and updates an existing documentation file to include that new fragment in the Sets section. No code or behavior changes.

Changes

Cohort / File(s) Summary
New mapping doc
docs/source/db_model_comparison.rst
New comprehensive documentation mapping database tables to model sets, parameters, model elements, and notes across domains (time, geography, technology, commodity, capacity, cost, efficiency, demand, emission, storage, operations, reserve margin, policy, construction, outputs, and internal structures).
Doc include update
docs/source/mathematical_formulation.rst
Inserted an include directive to reference the new db_model_comparison.rst fragment in the Sets section (two insertion points).

Sequence Diagram(s)

(omitted — changes are documentation-only and do not introduce new runtime control flow)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks

✅ 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 accurately describes the main change: adding documentation that maps between code elements and database tables across two documentation files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ef6acb and 43d4c8e.

📒 Files selected for processing (2)
  • docs/source/db_model_comparison.rst
  • docs/source/mathematical_formulation.rst
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T14:37:44.260Z
Learnt from: idelder
Repo: TemoaProject/temoa PR: 180
File: temoa/components/limits.py:632-647
Timestamp: 2025-10-30T14:37:44.260Z
Learning: In the `limit_tech_input_split_average_constraint` function in `temoa/components/limits.py`, there was a bug where the `total_inp` summation incorrectly used the fixed variable `i` instead of the loop variable `S_i` when indexing `processOutputsByInput` and calling `get_variable_efficiency`. This pattern should be watched for in similar aggregation constraints: when summing over a loop variable like `S_i`, all references within that loop should use `S_i`, not a fixed outer variable.

Applied to files:

  • docs/source/db_model_comparison.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: setup and test (windows-latest, 3.13)
  • GitHub Check: setup and test (macos-latest, 3.13)
  • GitHub Check: setup and test (windows-latest, 3.12)
  • GitHub Check: setup and test (macos-latest, 3.12)
🔇 Additional comments (1)
docs/source/mathematical_formulation.rst (1)

144-144: LGTM — Include directive correctly placed.

The include directive properly embeds the new database mapping documentation early in the Sets section, allowing readers to understand table-to-element mappings before engaging with detailed set definitions. Placement and syntax are correct.


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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ddcf8b and 7ef6acb.

📒 Files selected for processing (2)
  • docs/source/db_model_comparison.rst
  • docs/source/mathematical_formulation.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: setup and test (windows-latest, 3.12)
  • GitHub Check: setup and test (macos-latest, 3.12)
  • GitHub Check: setup and test (windows-latest, 3.13)
🔇 Additional comments (4)
docs/source/mathematical_formulation.rst (1)

143-147: Include directive placement and syntax look good.

The relative path is correct since both files are in the same directory. The external RST fragment will be properly expanded during documentation build. Note that the "New tables:" and "New tables End." text are informal markers; they won't generate formal RST elements but serve as visual delimiters.

docs/source/db_model_comparison.rst (3)

1-309: Excellent documentation structure and comprehensive mappings.

The new documentation file is well-organized with clear section hierarchy, consistent CSV table formatting, and appropriate use of RST roles (:math:, :code:, :header:, :widths:). The mappings between database tables and model elements are systematically categorized, and the "Notes" column provides helpful context for each mapping. The summary statistics block (lines 314-318) offers useful reference information.


299-309: "Database Tables Without Direct Model Mapping" section aids clarity.

This section (lines 299-309) appropriately documents tables like myopic_efficiency, time_manual, and sector_label that don't have direct model mappings. This helps users understand the full database schema and identifies less-commonly-used tables, which improves documentation completeness.


51-51: The line 51 use of tech_uncap is correct and matches the actual codebase definition. Verification of the Python codebase shows tech_uncap is consistently defined and used throughout (e.g., temoa/core/model.py:266, temoa/types/model_types.py:181). No instances of tech_unlim_cap exist in the model code, so if the existing documentation uses that variant, the existing documentation is outdated—not the new mapping document.

Comment on lines +186 to +188
":math:`\text{TISA}_{r,i,t}`", "limit_tech_input_split_annual", "limit_tech_input_split_annual", "average annual technology input fuel ratio; annual tech input splits"
":math:`\text{TOS}_{r,t,o}`", "limit_tech_output_split", "limit_tech_output_split", "technology output fuel ratio at time slice level; tech output split constraints"
":math:`\text{TISA}_{r,i,t}`", "limit_tech_output_split_annual", "limit_tech_output_split_annual", "average annual technology output fuel ratio; annual tech output splits"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Clarify mathematical notation for output split average parameter.

Line 188 repeats the same mathematical notation \text{TISA}_{r,i,t} for limit_tech_output_split_annual as line 186 uses for input split. Consider clarifying whether output split should use different notation (e.g., TOSA for output vs TISA for input) to maintain clarity, or add a note explaining why they share notation. This improves alignment with the parameter descriptions in the existing mathematical formulation documentation.

🤖 Prompt for AI Agents
In docs/source/db_model_comparison.rst around lines 186 to 188, the LaTeX math
notation for the annual output-split parameter is duplicated as
\text{TISA}_{r,i,t} (same as the input-split), which is confusing; change the
math string for limit_tech_output_split_annual to a distinct notation (e.g.,
\text{TOSA}_{r,t,o} or \text{TOSA}_{r,o,t} consistent with the ordering used for
limit_tech_output_split) and update the accompanying short description to
mention "average annual technology output fuel ratio; annual tech output
splits", or alternatively add a brief note explaining why input and output
annual splits intentionally share the same notation if that was deliberate.

Co-authored-by: jdecarolis <jdecarolis@cmu.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants