Uh oh!
There was an error while loading. Please reload this page.
feat: better notebooks, removed obsolete warnings and remove progres bars when not logging solve to console - #536
Conversation
Caution Review failedThe pull request is closed. WalkthroughAdds status-based operational constraints for boilers via StatusParameters, consolidates and compacts notebook outputs and cell source representations, suppresses solver console output in notebook mode, and threads a progress flag through the solve pipeline and FlowSystem logging. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FlowSystem
participant Boiler as MainBoiler\n(StatusParameters)
participant Solver
participant Solution
User->>FlowSystem: build system (includes Boiler with StatusParameters)
FlowSystem->>Boiler: register constraints (min_uptime, min_downtime, startup_costs)
alt Constrained run
User->>FlowSystem: solve(constrained)
FlowSystem->>Solver: solve(progress=CONFIG.Solving.log_to_console)
Solver->>Boiler: enforce status constraints during optimization
Solver->>Solution: return constrained solution (statuses, costs)
else Baseline run
User->>FlowSystem: solve(baseline)
FlowSystem->>Solver: solve(progress=CONFIG.Solving.log_to_console)
Solver->>Solution: return baseline solution
end
Solution->>FlowSystem: extract status-aware outputs (on/off, heatmaps)
FlowSystem->>User: report consolidated summaries and constraint overhead
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/notebooks/03-investment-optimization.ipynb(4 hunks)docs/notebooks/04-operational-constraints.ipynb(9 hunks)docs/notebooks/05-multi-carrier-system.ipynb(3 hunks)docs/notebooks/06a-time-varying-parameters.ipynb(8 hunks)docs/notebooks/09-plotting-and-data-access.ipynb(1 hunks)docs/notebooks/10-transmission.ipynb(10 hunks)flixopt/config.py(2 hunks)flixopt/flow_system.py(2 hunks)flixopt/optimization.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
flixopt/optimization.py (1)
flixopt/config.py (2)
CONFIG(181-893)Solving(522-545)
flixopt/flow_system.py (1)
flixopt/config.py (2)
CONFIG(181-893)Solving(522-545)
⏰ 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). (5)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.14)
- GitHub Check: test (3.11)
- GitHub Check: Build documentation
🔇 Additional comments (7)
flixopt/config.py (2)
796-796: LGTM! Clear documentation of behavior.The comment accurately describes the configuration change below and improves code readability.
821-823: LGTM! Sensible defaults for notebook environments.Disabling solver console output and main results logging in notebooks reduces cell clutter while still allowing users to manually enable these via
CONFIG.Solving.log_to_console = Trueif needed for debugging.This aligns with the progress parameter changes in
flixopt/optimization.py(line 245) andflixopt/flow_system.py(line 1329) that respect this configuration flag.flixopt/flow_system.py (2)
600-600: LGTM! Appropriate log level for auto-correcting behavior.Downgrading from WARNING to INFO is correct since
connect_and_transform()is called automatically. This isn't an error condition but normal behavior, so INFO level is more appropriate and won't unnecessarily alarm users.
1329-1329: LGTM! Consistent progress control with Optimization.solve().The addition of
progress=CONFIG.Solving.log_to_consolemaintains consistency with the identical change inflixopt/optimization.py(line 245), ensuring uniform progress bar behavior across both the new (FlowSystem.solve()) and deprecated (Optimization.solve()) APIs.docs/notebooks/06a-time-varying-parameters.ipynb (1)
53-54: LGTM! Standard encoding practices for notebook compatibility.All changes are Unicode character escape sequences (° →
\u00b0, → →\u2192, € →\u20ac) that improve cross-platform compatibility and version control handling. No functional changes to the notebook's logic or outputs.Also applies to: 85-85, 117-117, 151-151, 171-171, 196-196, 252-252, 286-286
flixopt/optimization.py (1)
245-245: linopy Model.solve() correctly accepts the progress parameter.The progress parameter is a boolean option that controls whether to show a progress bar of writing the lp file, with the default being None (which shows the progress bar if the model has more than 10000 variables and constraints). Passing
CONFIG.Solving.log_to_consoleto control progress bar visibility is a valid and appropriate use of this parameter.docs/notebooks/04-operational-constraints.ipynb (1)
143-159: The fuel_flow size requirement when using status_parameters is documented in the v5.0.0 CHANGELOG as a breaking change and validation constraint. The notebook example correctly includes size=600 with an explanatory comment. Validation will catch violations with a clear error message, guiding users who may not have consulted the migration guide.
| "source": "# Run the generation script (only needed once, or to regenerate)\n!python data/generate_example_systems.py > /dev/null 2>&1", | ||
| "outputs": [], | ||
| "execution_count": null |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Check if the notebook file exists and view the relevant linesif [ -f"docs/notebooks/09-plotting-and-data-access.ipynb" ];thenecho"=== Notebook file found ==="# Use jq to parse the JSON notebook and find the cell around lines 66-68
jq '.cells[] | select(.source | tostring | contains("generate_example_systems")) | {source, outputs, execution_count}' docs/notebooks/09-plotting-and-data-access.ipynb 2>/dev/null ||echo"JQ parsing failed, trying alternative method"echo""echo"=== Raw JSON excerpt ==="# Show raw portion to verify structure
jq '.cells[20:25]' docs/notebooks/09-plotting-and-data-access.ipynb 2>/dev/null | head -100
fi# Check if the data generation script existsif [ -f"data/generate_example_systems.py" ];thenecho"=== Data generation script found ==="
wc -l data/generate_example_systems.py
head -30 data/generate_example_systems.py
elseecho"=== Data generation script NOT found ==="fiRepository: flixOpt/flixopt
Length of output: 8946
🏁 Script executed:
#!/bin/bash# Find the notebook file
find . -name "09-plotting-and-data-access.ipynb" -type f
# If found, examine its contentif [ -f"docs/notebooks/09-plotting-and-data-access.ipynb" ];thenecho"=== Checking notebook structure ==="
jq -r '.cells | length' docs/notebooks/09-plotting-and-data-access.ipynb
echo""echo"=== Looking for cells with generate_example_systems ==="
jq -r '.cells[] | select(.cell_type=="code") | .source[] | select(contains("generate_example_systems"))?' docs/notebooks/09-plotting-and-data-access.ipynb 2>/dev/null ||echo"No match found with simple grep"# Alternative: show cell sources to find the right oneecho""echo"=== All code cells with 'python' keyword ==="
jq -r '.cells[] | select(.cell_type=="code" and (.source | tostring | contains("python"))) | "\(.source)"' docs/notebooks/09-plotting-and-data-access.ipynb | head -50
fiRepository: flixOpt/flixopt
Length of output: 145
🏁 Script executed:
#!/bin/bashcd /root/repo 2>/dev/null ||cd.# Check file existsif [ !-f"docs/notebooks/09-plotting-and-data-access.ipynb" ];thenecho"File not found at docs/notebooks/09-plotting-and-data-access.ipynb"# Try to find it
find . -name "*09-plotting*" -type f 2>/dev/null | head -5
exit 1
fiecho"=== File found ==="# Use python to parse JSON and find the cell
python3 << 'EOF'import jsonimport syswith open('docs/notebooks/09-plotting-and-data-access.ipynb', 'r') as f: nb = json.load(f)# Find cells with generate_example_systemsfor i, cell in enumerate(nb['cells']): if cell.get('cell_type') == 'code': source = ''.join(cell.get('source', [])) if 'generate_example_systems' in source: print(f"\n=== Cell {i} ===") print(f"Source: {source}") print(f"Outputs: {cell.get('outputs', [])}") print(f"Execution count: {cell.get('execution_count')}") breakelse: print("No cell with 'generate_example_systems' found") print(f"Total cells: {len(nb['cells'])}")EOFRepository: flixOpt/flixopt
Length of output: 145
🏁 Script executed:
#!/bin/bash# Direct check of the notebook file
python3 << 'PYEOF'import jsontry: with open('docs/notebooks/09-plotting-and-data-access.ipynb') as f: nb = json.load(f) # Search for the cell with generate_example_systems for cell in nb['cells']: if cell.get('cell_type') == 'code': src = ''.join(cell.get('source', [])) if 'generate_example_systems' in src: print("FOUND CELL:") print(src) print("\n---") print(f"Outputs: {len(cell.get('outputs', []))} items") print(f"Execution count: {cell.get('execution_count')}") break else: print("Cell not found - checking data script exists") import os print(f"Script exists: {os.path.exists('data/generate_example_systems.py')}")except Exception as e: print(f"Error: {e}")PYEOFRepository: flixOpt/flixopt
Length of output: 145
Remove output redirection or add explicit error handling; the data generation script is missing and failures are hidden.
The script data/generate_example_systems.py does not exist in the repository. When the command runs and fails, the > /dev/null 2>&1 redirection hides the error completely, leaving users unaware that the setup step failed. The cell's null execution_count suggests it may never have executed successfully.
Either:
- Remove the redirection to surface errors:
!python data/generate_example_systems.py(or confirm if this script is expected to exist) - Or wrap in Python with explicit error handling:
import subprocess; result = subprocess.run(...); result.check_returncode()
🤖 Prompt for AI Agents
In docs/notebooks/09-plotting-and-data-access.ipynb lines 66-68 the notebook
cell runs a non-existent script with output redirected to /dev/null which hides
failures; update the cell to either (a) remove the redirection so errors are
visible and confirm the script path/name is correct, or (b) replace the shell
invocation with a short Python wrapper that runs the script via subprocess.run
and calls check_returncode() to raise on failure so the notebook surfaces
errors; ensure the cell is executed and execution_count is set after
verification.
Uh oh!
There was an error while loading. Please reload this page.
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.