Fixing regressions in pdf documentation building #243
Conversation
WalkthroughUpdated documentation and build scripts: changed Sphinx build output directory, made the Changes
Sequence Diagram(s)(omitted — changes are documentation and small Makefile control flow that do not meet diagram criteria) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (10)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-30T14:37:44.260ZApplied to files:
⏰ 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)
🔇 Additional comments (12)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @docs/Makefile:
- Around line 8-10: The Makefile has an outdated commented assignment and note
next to the BUILDDIR variable; update or remove the obsolete comment lines
referencing "/tmp/TemoaDocumentationBuild" so the comment accurately reflects
that BUILDDIR is set to "_build" (Sphinx default) and no longer to "build" or
the temp path; locate the BUILDDIR definition and either delete the two
commented lines or replace them with a short, accurate comment explaining that
"_build" is the Sphinx output directory.
In @docs/README.md:
- Around line 44-46: Update the README text to clarify that latexmk is
recommended but not required: change the sentence that currently says "you'll
need LaTeX and `latexmk` installed" to state "you'll need LaTeX installed;
`latexmk` is recommended for automatic PDF generation" and mention the fallback
to manual pdflatex is available (as implemented in the Makefile rules that fall
back to pdflatex when `latexmk` is unavailable).
In @temoa/components/limits.py:
- Line 257: Update the Theta docstring labels so they match the actual
constraint identifiers: replace occurrences of "limit_growth_capacity" and
"limit_degrowth_capacity" with "limit_growth_new_capacity" and
"limit_degrowth_new_capacity", and replace "limit_growth_capacityDelta" and
"limit_degrowth_capacityDelta" (camelCase) with
"limit_growth_new_capacity_delta" and "limit_degrowth_new_capacity_delta" so the
Theta_{...} labels align with the constraint names
limit_growth_new_capacity_constraint, limit_degrowth_new_capacity_constraint,
limit_growth_new_capacity_delta_constraint, and
limit_degrowth_new_capacity_delta_constraint in the module.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/source/images/graphviz_examples/results1990.pdfis excluded by!**/*.pdfdocs/source/images/results1990.pdfis excluded by!**/*.pdfdocs/source/images/results1990.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
docs/Makefiledocs/README.mddocs/source/Documentation.rstdocs/source/computational_implementation.rstdocs/source/mathematical_formulation.rstdocs/source/quick_start.rstdocs/source/unit_checking.rsttemoa/components/limits.pytemoa/components/operations.pytemoa/components/storage.py
🧰 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:
temoa/components/limits.py
🔇 Additional comments (11)
docs/source/quick_start.rst (1)
85-85: LaTeX/PDF compatibility fix: Standardize quotes around config.Replacing smart quotes with ASCII straight quotes is the correct approach for ensuring LaTeX/PDF builders handle documentation correctly without escaping issues.
docs/source/mathematical_formulation.rst (1)
1064-1064: LaTeX best practice: Escape underscores in subscript text notation.The change from
\Theta_{\text{cost_invest}}to\Theta_{\text{cost\_invest}}properly escapes underscores within LaTeX text mode, preventing PDF build failures. This follows standard LaTeX conventions for literal underscores.temoa/components/operations.py (3)
284-284: LaTeX best practice: Escape underscores in docstring subscript notation.The ramp_up_day constraint docstring properly escapes underscores in the mathematical notation
\Theta_{\text{ramp\_up\_day}}, ensuring PDF documentation builders handle the LaTeX correctly.
387-387: LaTeX best practice: Escape underscores in docstring subscript notation.The ramp_down_day constraint docstring properly escapes underscores in the mathematical notation
\Theta_{\text{ramp\_down\_day}}, consistent with LaTeX formatting for PDF generation.
1-593: No action required. All constraint docstrings with Theta notation acrosstemoa/components/have been consistently updated with properly escaped underscores in\text{}blocks. Coverage spans operations.py, limits.py, storage.py, reserves.py, geography.py, costs.py, and emissions.py.docs/source/computational_implementation.rst (1)
381-392: Status marker standardization improves PDF rendering and accessibility.Replacing emoji markers (
⚠️) with text-based markers ([!]) is appropriate for:
- Improving LaTeX/PDF rendering consistency
- Enhancing accessibility across platforms
- Aligning with documentation build goals
The changes are uniform and do not affect content or functionality.
docs/source/Documentation.rst (1)
80-89: Image files exist and wildcard pattern is correct.The figure directive change to use the wildcard pattern (
images/results1990.*) is valid. Bothresults1990.pdfandresults1990.svgexist indocs/source/images/, allowing Sphinx to select the appropriate format for each builder (SVG for HTML, PDF for LaTeX). No issues found.docs/source/unit_checking.rst (1)
103-353: LGTM! ASCII marker replacements improve PDF compatibility.The replacement of Unicode emojis (✅, ❌,
⚠️ ) with ASCII markers ([V], [X], [!]) and arrow symbols (→) with "->" throughout the documentation enhances LaTeX/PDF rendering compatibility while maintaining semantic clarity. These changes directly address the PR objective of fixing PDF documentation build regressions.temoa/components/storage.py (1)
506-512: LGTM! Proper LaTeX underscore escaping fixes PDF rendering.The underscore escaping in the docstring's LaTeX math notation (
\text{C2A}→\text{C2A},limit_storage_fraction→limit\_storage\_fraction, etc.) correctly addresses LaTeX compilation issues. Unescaped underscores in LaTeX text mode cause parse errors, so these changes are essential for PDF documentation generation.docs/README.md (1)
51-51: LGTM! Simplified command aligns with new Makefile target.The change from direct
sphinx-buildinvocation touv run make latexpdfis correct. The Makefile now handles the conditional logic for latexmk detection and fallback, simplifying the user experience.docs/Makefile (1)
111-126: Good implementation of latexmk detection with fallback.The conditional logic correctly detects latexmk availability and falls back to manual pdflatex with two passes for reference resolution. The LATEXOPTS are appropriate for CI/automation contexts.
One minor consideration: Line 120's
|| trueallows the first pdflatex pass to continue on errors, which is intentional for unresolved references. However, the-halt-on-errorflag in PDFLATEX should still catch genuine compilation errors before they're masked. The second pass (lines 122-125) runs without|| true, ensuring real errors aren't silently ignored.
| \le LR_{r, t} | ||
| \forall \{r, t\} \in \Theta_{\text{limit_resource}}""" | ||
| \forall \{r, t\} \in \Theta_{\text{limit\_resource}}""" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and verify it exists
ls -la temoa/components/limits.py 2>/dev/null && wc -l temoa/components/limits.pyRepository: TemoaProject/temoa
Length of output: 170
🏁 Script executed:
# Check lines around 1176, 1185, 1056, 1064 to verify constraint labels
sed -n '1050,1070p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 1014
🏁 Script executed:
sed -n '1170,1190p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 961
🏁 Script executed:
# Also check the function definitions to verify names
rg -n "def limit_growth|def limit_degrowth" temoa/components/limits.pyRepository: TemoaProject/temoa
Length of output: 824
🏁 Script executed:
# Verify underscore escaping in \text{} blocks across the file
rg -n '\\text\{[^}]*_' temoa/components/limits.py | head -20Repository: TemoaProject/temoa
Length of output: 1631
🏁 Script executed:
# Check if the escaping is using \_ properly
rg -n '\\text\{[^}]*\\_' temoa/components/limits.py | head -20Repository: TemoaProject/temoa
Length of output: 1631
🏁 Script executed:
# Read the full docstring context for lines 1034-1070 (limit_growth_new_capacity function)
sed -n '1034,1070p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 1888
🏁 Script executed:
# Read the full docstring context for lines 1151-1190 (limit_growth_new_capacity_delta function)
sed -n '1151,1190p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 2104
🏁 Script executed:
# Check the constraint indices function names to understand naming convention
sed -n '156,175p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 779
🏁 Script executed:
# Verify if the labels in \Theta correspond to actual constraint indices/set names
rg -n "limit_growth_capacity_indices|limit_growth_new_capacity_indices|limit_growth_new_capacity_delta_indices" temoa/components/limits.py | head -10Repository: TemoaProject/temoa
Length of output: 258
🏁 Script executed:
# Check if constraint sets are actually defined with these names in the model
rg -n "limit_growth_capacity|limit_growth_new_capacity|limit_growth_capacityDelta|limit_growth_new_capacity_delta" temoa/components/limits.py | grep -E "(indices|def |\.declare|\.add)" | head -30Repository: TemoaProject/temoa
Length of output: 524
🏁 Script executed:
# Check model declarations to see what constraint sets actually exist
rg -n "\.add\(|\.declare\(" temoa/components/limits.py | grep -i "capacity" | head -20Repository: TemoaProject/temoa
Length of output: 44
🏁 Script executed:
# Check what constraint objects are actually declared in the model
rg -n "model\.limit_growth|model\.limit_degrowth" temoa/components/limits.py | grep -E "constraint|Constraint" | head -20Repository: TemoaProject/temoa
Length of output: 44
🏁 Script executed:
# Get full context of how constraints are defined/added to model
sed -n '1080,1130p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 2670
🏁 Script executed:
# Find where constraint rules are registered/called
rg -n "constraint_rule|add_component" temoa/components/limits.py | grep -i "capacity" | head -30Repository: TemoaProject/temoa
Length of output: 381
🏁 Script executed:
# Look for the actual constraint registration to see what names are used
rg -n "model\..*constraint|\.add_component" temoa/components/limits.py | head -40Repository: TemoaProject/temoa
Length of output: 44
🏁 Script executed:
# Check the full context around the constraint rule definitions
sed -n '1020,1045p' temoa/components/limits.py | cat -nRepository: TemoaProject/temoa
Length of output: 1460
🏁 Script executed:
# Find where constraints are declared/added to the model
find temoa -name "*.py" -exec grep -l "limit_growth_new_capacity\|limit_degrowth_new_capacity" {} \;Repository: TemoaProject/temoa
Length of output: 184
🏁 Script executed:
# Check the model initialization or pyomo declarations
rg -n "Constraint|Declaration|add_component" temoa/components/ --type py -A 2 | grep -E "limit_growth|limit_degrowth" | head -40Repository: TemoaProject/temoa
Length of output: 44
🏁 Script executed:
# Look for constraint declarations in a model/declarations file
find temoa -name "*model*" -o -name "*declaration*" | grep -E "\.(py|txt)$"Repository: TemoaProject/temoa
Length of output: 211
🏁 Script executed:
# Check how constraints are declared in model.py
rg -n "limit_growth_new_capacity|limit_degrowth_new_capacity|limit_growth_capacity" temoa/core/model.py -B 2 -A 2 | head -80Repository: TemoaProject/temoa
Length of output: 2984
🏁 Script executed:
# Check for constraint declarations in model.py more broadly
rg -n "\.add_component\(|Constraint" temoa/core/model.py | grep -i "limit" | head -40Repository: TemoaProject/temoa
Length of output: 1792
🏁 Script executed:
# Final verification: confirm the constraint labels referenced in docstrings don't match constraint names
echo "=== Constraint declarations in model.py ==="
rg -n "limit_growth_new_capacity_constraint|limit_degrowth_new_capacity_constraint|limit_growth_new_capacity_delta" temoa/core/model.py | grep "Constraint("
echo "=== Docstring labels in limits.py ==="
rg -n "Theta.*limit_growth_capacity|Theta.*limit_degrowth_capacity|Theta.*capacityDelta" temoa/components/limits.pyRepository: TemoaProject/temoa
Length of output: 546
🏁 Script executed:
# Verify one more time if there's any comment or explanation in the code about label naming
sed -n '1050,1070p' temoa/components/limits.py | grep -A 5 -B 5 "limit_growth_capacity\|limit_degrowth_capacity"Repository: TemoaProject/temoa
Length of output: 44
🏁 Script executed:
# Check if there are any comments or explanations for the capacityDelta naming
sed -n '1170,1190p' temoa/components/limits.py | grep -A 5 -B 5 "capacityDelta"Repository: TemoaProject/temoa
Length of output: 769
Update constraint set labels in docstrings to match actual constraint names.
The underscore escaping in \text{} blocks is correct and necessary for LaTeX rendering.
However, the constraint set labels in the docstring Theta notation don't match the actual constraint names:
- Lines 1056, 1064: Labels show
limit_growth_capacityandlimit_degrowth_capacity, but the constraints arelimit_growth_new_capacity_constraintandlimit_degrowth_new_capacity_constraint. Labels should belimit_growth_new_capacityandlimit_degrowth_new_capacity. - Lines 1176, 1185: Labels show
limit_growth_capacityDeltaandlimit_degrowth_capacityDelta(camelCase), but constraints arelimit_growth_new_capacity_delta_constraintandlimit_degrowth_new_capacity_delta_constraint. Labels should belimit_growth_new_capacity_deltaandlimit_degrowth_new_capacity_delta(snake_case).
🤖 Prompt for AI Agents
In @temoa/components/limits.py around line 257, Update the Theta docstring
labels so they match the actual constraint identifiers: replace occurrences of
"limit_growth_capacity" and "limit_degrowth_capacity" with
"limit_growth_new_capacity" and "limit_degrowth_new_capacity", and replace
"limit_growth_capacityDelta" and "limit_degrowth_capacityDelta" (camelCase) with
"limit_growth_new_capacity_delta" and "limit_degrowth_new_capacity_delta" so the
Theta_{...} labels align with the constraint names
limit_growth_new_capacity_constraint, limit_degrowth_new_capacity_constraint,
limit_growth_new_capacity_delta_constraint, and
limit_degrowth_new_capacity_delta_constraint in the module.
ParticularlyPythonicBS
force-pushed
the
docs/pdf_builder
branch
from
dcb41a2 to
0d6ae91
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @docs/source/quick_start.rst:
- Around line 85-89: The file contains unresolved Git conflict markers (<<<<<<<
HEAD, =======, >>>>>>> dcb41a2) around the sentence starting "To run the model,
a configuration ..."—remove the conflict markers and keep the single-quoted form
('config') to match LaTeX-compatibility and project style; ensure the final line
reads: To run the model, a configuration ('config') file is needed. An (and
remove any leftover marker lines).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/source/images/graphviz_examples/results1990.pdfis excluded by!**/*.pdfdocs/source/images/results1990.pdfis excluded by!**/*.pdfdocs/source/images/results1990.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
docs/Makefiledocs/README.mddocs/source/Documentation.rstdocs/source/computational_implementation.rstdocs/source/mathematical_formulation.rstdocs/source/quick_start.rstdocs/source/unit_checking.rsttemoa/components/limits.pytemoa/components/operations.pytemoa/components/storage.py
🧰 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:
temoa/components/limits.pytemoa/components/operations.pytemoa/components/storage.py
⏰ 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 (macos-latest, 3.13)
- GitHub Check: setup and test (macos-latest, 3.12)
- GitHub Check: setup and test (windows-latest, 3.12)
🔇 Additional comments (10)
temoa/components/limits.py (1)
257-257: LGTM! Underscore escaping is correct for LaTeX PDF rendering.The underscore escaping in
\text{}blocks throughout the constraint docstrings is necessary and correctly implemented for LaTeX PDF generation. The pattern\text{constraint\_name}properly escapes underscores to prevent LaTeX interpretation errors.These documentation-only changes align with the PR objective of fixing regressions in PDF documentation building.
Also applies to: 313-313, 460-460, 465-465, 522-522, 528-528, 711-711, 746-746, 823-823, 935-935, 943-943, 1056-1056, 1064-1064, 1176-1176, 1185-1185, 1301-1301, 1305-1305, 1381-1381
docs/source/computational_implementation.rst (1)
381-392: LGTM! Status marker format update is consistent.The status marker format change from "
⚠️ untested in v4.0" to "([!] untested in v4.0)" is consistently applied across all affected module entries. This documentation formatting update likely improves LaTeX PDF rendering compatibility.docs/source/Documentation.rst (1)
80-80: LGTM! Image path update adds format flexibility.The figure path change from
images/graphviz_examples/results1990.svgtoimages/results1990.*uses a wildcard pattern that allows Sphinx to automatically select the appropriate image format (SVG, PNG, etc.) based on the output target (HTML vs. PDF). This improves documentation build flexibility.temoa/components/operations.py (1)
284-284: LGTM! Underscore escaping is correct for LaTeX rendering.The underscore escaping in the Theta notation (
\text{ramp\_up\_day}and\text{ramp\_down\_day}) is correctly implemented for LaTeX PDF generation, consistent with the escaping pattern used throughout the codebase.Also applies to: 387-387
docs/source/mathematical_formulation.rst (1)
1067-1067: LGTM! LaTeX subscript syntax corrected.The change from
\Theta_\text{cost_invest}to\Theta_{\text{cost\_invest}}properly applies braces around the entire subscript and escapes the underscore. This ensures correct LaTeX parsing and rendering in PDF documentation.docs/source/unit_checking.rst (1)
103-353: LGTM! Emoji replaced with LaTeX-compatible text markers.The systematic replacement of emoji-based correctness indicators (✅/❌) with bracketed text markers ([V]/[X]) throughout the documentation improves LaTeX/PDF compatibility. Emoji characters can cause rendering issues in PDF generation toolchains, and this change ensures consistent, reliable output across all documentation formats.
temoa/components/storage.py (1)
506-512: LGTM! LaTeX formatting corrected for PDF documentation.The docstring updates properly escape underscores in the mathematical notation (line 512:
Theta_{limit\_storage\_fraction}) and improve the formatting of mathematical expressions. Unescaped underscores in LaTeX subscripts can cause compilation errors in PDF generation. These changes ensure the constraint documentation renders correctly in the generated PDF.docs/README.md (1)
44-51: LGTM! Documentation accurately reflects the Makefile's fallback logic.The updated wording correctly describes that LaTeX is required while latexmk is recommended, which aligns with the conditional fallback logic implemented in the Makefile (lines 109-124). Adding TeX Live as an alternative to MiKTeX provides users with more options for LaTeX installation.
The previous review concern about clarifying latexmk as recommended vs required has been addressed by this wording.
docs/Makefile (2)
8-8: LGTM! BUILDDIR updated to Sphinx convention.Changing BUILDDIR to
_buildaligns with Sphinx's default directory naming convention and modern best practices.
109-124: LGTM! Well-implemented fallback for missing latexmk.The conditional logic properly handles environments where latexmk is unavailable by:
- Checking for latexmk availability with
command -v- Falling back to manual pdflatex with appropriate options
- Running pdflatex twice to ensure references are resolved
- Providing clear user feedback about the fallback
This implementation aligns with the updated documentation in README.md that describes latexmk as recommended rather than required.
ParticularlyPythonicBS
force-pushed
the
docs/pdf_builder
branch
from
0d6ae91 to
48d339b
Compare
Uses make target instead of direct sphinx builder and fixes latex specific formatting and compatibility issues.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.