-
Notifications
You must be signed in to change notification settings - Fork 9
Feature/300 piecewiseeffects for flows #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c402d57
e707d0a
3863102
ca6424f
69791d7
10635f8
a0a5785
3a88a59
c4db3b7
12fdae7
bcca8a2
1914b70
ed8a8f2
cc25934
6d73883
5ef56a5
d5e8e24
14505bb
dd6ea15
fb13157
f7658dd
380907e
ca1a9f0
109c6ab
454fe19
bd40414
fc71d85
ffd6b92
5ae6b05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -142,11 +142,58 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| python-version: ${{ env.PYTHON_VERSION }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Extract release notes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check if pre-release | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: prerelease | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "${{ github.ref }}" =~ (alpha|beta|rc) ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "is_prerelease=true" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "✅ Detected pre-release" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "is_prerelease=false" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "✅ Detected stable release" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Create release notes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VERSION=${GITHUB_REF#refs/tags/v} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Extracting release notes for version: $VERSION" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| python scripts/extract_release_notes.py $VERSION > current_release_notes.md | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Creating release notes for version: $VERSION" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "${{ steps.prerelease.outputs.is_prerelease }}" == "true" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "📝 Generating pre-release notes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat > current_release_notes.md << EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Pre-release $VERSION | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This is a pre-release version for testing and feedback. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Installation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| \`\`\`bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pip install flixopt==$VERSION --pre | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| \`\`\` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## What's Changed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| See the [unreleased section](https://github.com/flixOpt/flixopt/blob/main/CHANGELOG.md#unreleased) in the changelog for upcoming features and changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Feedback | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Please report any issues or feedback on [GitHub Issues](https://github.com/flixOpt/flixopt/issues). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+161
to
+179
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Heredoc indentation makes headings render as code blocks in release notes YAML indentation is persisted inside the heredoc, so “# Pre-release …” is emitted with 10 leading spaces and shows up as a code block on GitHub. Strip the indent or use a tab-stripping heredoc. Minimal fix (strip the 10-space indent after write): if [[ "${{ steps.prerelease.outputs.is_prerelease }}" == "true" ]]; then
echo "📝 Generating pre-release notes"
- cat > current_release_notes.md << EOF
+ cat > current_release_notes.md << EOF
# Pre-release $VERSION
This is a pre-release version for testing and feedback.
## Installation
\`\`\`bash
pip install flixopt==$VERSION --pre
\`\`\`
## What's Changed
See the [unreleased section](https://github.com/flixOpt/flixopt/blob/main/CHANGELOG.md#unreleased) in the changelog for upcoming features and changes.
## Feedback
Please report any issues or feedback on [GitHub Issues](https://github.com/flixOpt/flixopt/issues).
EOF
+ # Remove YAML indentation (10 spaces) to render Markdown headings correctly
+ sed -i 's/^ //' current_release_notes.mdAlternative (no sed): use a tab-indented heredoc with stripping: - cat > current_release_notes.md << EOF
+ cat > current_release_notes.md <<- EOF…and prefix all heredoc content lines with a single tab instead of spaces. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "📝 Extracting release notes from changelog" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if python scripts/extract_release_notes.py "$VERSION" > current_release_notes.md 2>/dev/null; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "✅ Successfully extracted release notes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "⚠️ No release notes found, using fallback" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat > current_release_notes.md << EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Release $VERSION | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| See the [full changelog](https://github.com/flixOpt/flixopt/blob/main/CHANGELOG.md) for details. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Generated release notes:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "========================" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat current_release_notes.md | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "========================" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Create GitHub Release | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: softprops/action-gh-release@v2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,8 @@ | |
| from .config import CONFIG | ||
| from .core import NumericData, NumericDataTS, PlausibilityError, Scalar, TimeSeriesCollection | ||
| from .effects import EffectValuesUser | ||
| from .features import InvestmentModel, OnOffModel, PreventSimultaneousUsageModel | ||
| from .interface import InvestParameters, OnOffParameters | ||
| from .features import InvestmentModel, OnOffModel, PiecewiseEffectsPerFlowHourModel, PreventSimultaneousUsageModel | ||
| from .interface import InvestParameters, OnOffParameters, PiecewiseEffectsPerFlowHour | ||
| from .structure import Element, ElementModel, SystemModel, register_class_for_io | ||
|
|
||
| if TYPE_CHECKING: | ||
|
|
@@ -159,6 +159,7 @@ def __init__( | |
| relative_minimum: NumericDataTS = 0, | ||
| relative_maximum: NumericDataTS = 1, | ||
| effects_per_flow_hour: Optional[EffectValuesUser] = None, | ||
| piecewise_effects_per_flow_hour: Optional[PiecewiseEffectsPerFlowHour] = None, | ||
| on_off_parameters: Optional[OnOffParameters] = None, | ||
| flow_hours_total_max: Optional[Scalar] = None, | ||
| flow_hours_total_min: Optional[Scalar] = None, | ||
|
|
@@ -180,6 +181,7 @@ def __init__( | |
| def: :math:`load\_factor:= sumFlowHours/ (nominal\_val \cdot \Delta t_{tot})` | ||
| load_factor_max: maximal load factor (see minimal load factor) | ||
| effects_per_flow_hour: operational costs, costs per flow-"work" | ||
| piecewise_effects_per_flow_hour: piecewise relation between flow hours and effects | ||
| on_off_parameters: If present, flow can be "off", i.e. be zero (only relevant if relative_minimum > 0) | ||
| Therefore a binary var "on" is used. Further, several other restrictions and effects can be modeled | ||
| through this On/Off State (See OnOffParameters) | ||
|
|
@@ -207,6 +209,7 @@ def __init__( | |
| self.load_factor_max = load_factor_max | ||
| # self.positive_gradient = TimeSeries('positive_gradient', positive_gradient, self) | ||
| self.effects_per_flow_hour = effects_per_flow_hour if effects_per_flow_hour is not None else {} | ||
| self.piecewise_effects_per_flow_hour = piecewise_effects_per_flow_hour | ||
| self.flow_hours_total_max = flow_hours_total_max | ||
| self.flow_hours_total_min = flow_hours_total_min | ||
| self.on_off_parameters = on_off_parameters | ||
|
|
@@ -248,6 +251,8 @@ def transform_data(self, flow_system: 'FlowSystem'): | |
| self.effects_per_flow_hour = flow_system.create_effect_time_series( | ||
| self.label_full, self.effects_per_flow_hour, 'per_flow_hour' | ||
| ) | ||
| if self.piecewise_effects_per_flow_hour is not None: | ||
| self.piecewise_effects_per_flow_hour.transform_data(flow_system, self.label_full) | ||
| if self.on_off_parameters is not None: | ||
| self.on_off_parameters.transform_data(flow_system, self.label_full) | ||
| if isinstance(self.size, InvestParameters): | ||
|
|
@@ -398,6 +403,21 @@ def _create_shares(self): | |
| target='operation', | ||
| ) | ||
|
|
||
| if self.element.piecewise_effects_per_flow_hour is not None: | ||
| self.piecewise_effects = self.add( | ||
| PiecewiseEffectsPerFlowHourModel( | ||
| model=self._model, | ||
| label_of_element=self.label_of_element, | ||
| piecewise_origin=( | ||
| self.flow_rate.name, | ||
| self.element.piecewise_effects_per_flow_hour.piecewise_flow_rate, | ||
| ), | ||
| piecewise_shares=self.element.piecewise_effects_per_flow_hour.piecewise_shares, | ||
| zero_point=self.on_off.on if self.on_off is not None else False, | ||
| ), | ||
| ) | ||
| self.piecewise_effects.do_modeling() | ||
|
|
||
|
Comment on lines
+406
to
+420
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainZero-point handling risks infeasibility when On/Off is absent. Passing False forces “exactly one segment active,” disallowing zero unless 0 is inside a piece. That can make otherwise valid flows (min=0) infeasible. Recommend allowing zero by default when no On/Off is present. - zero_point=self.on_off.on if self.on_off is not None else False,
+ zero_point=self.on_off.on if self.on_off is not None else True,Would you prefer a Flow-level flag (e.g., piecewise_zero_allowed_when_off: bool = True) to make this explicit? 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Listing flixopt module files (top-level):"
ls -la flixopt || true
echo
echo "Search for Piecewise* classes / identifiers..."
rg -n --hidden --no-ignore-vcs "PiecewiseEffectsPerFlowHourModel" -S || true
rg -n --hidden --no-ignore-vcs "PiecewiseEffects" -S || true
rg -n --hidden --no-ignore-vcs "piecewise_effects_per_flow_hour" -S || true
rg -n --hidden --no-ignore-vcs "piecewise" -S || true
echo
echo "Search for 'zero_point' usages:"
rg -n --hidden --no-ignore-vcs "zero_point" -S || true
echo
echo "Show flixopt/elements.py around the snippet (lines ~390-430):"
if [ -f flixopt/elements.py ]; then
nl -ba flixopt/elements.py | sed -n '390,430p' || true
else
echo "flixopt/elements.py not found"
fi
echo
echo "Search for classes with 'Piecewise' in their name (to find model implementation):"
rg -n --hidden --no-ignore-vcs "class .*Piecewise" -S || true
echo
echo "Search for constructor parameters that mention zero_point (def __init__ ... zero_point):"
rg -n --hidden --no-ignore-vcs "def __init__\([^\)]*zero_point" -S || true
echo
echo "Search for 'on_off' near piecewise-related code:"
rg -n --hidden --no-ignore-vcs "on_off" -S || trueLength of output: 53460 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== features.py 820-920 ==="
sed -n '820,920p' flixopt/features.py || true
echo
echo "=== features.py 1040-1140 ==="
sed -n '1040,1140p' flixopt/features.py || true
echo
echo "=== elements.py 392-422 ==="
sed -n '392,422p' flixopt/elements.py || true
echo
echo "=== components.py 528-540 ==="
sed -n '528,540p' flixopt/components.py || true
echo
echo "=== tests/test_linear_converter.py 420-440 ==="
sed -n '420,440p' tests/test_linear_converter.py || trueLength of output: 11083 Allow zero when On/Off is absent — fix in PiecewiseModel, not by passing True at call sites Confirmed: call sites (e.g. flixopt/elements.py) pass False when no On/Off; PiecewiseModel.do_modeling treats non-True/non-variable as sum(insides) == 1, forcing a segment and making flows with min=0 infeasible. Switching call-sites to True would create an unconstrained binary zero_point (undesirable). Recommended minimal fix: in flixopt/features.py -> PiecewiseModel.do_modeling change the else branch from Would you prefer an explicit Flow-level flag (e.g. piecewise_zero_allowed_when_off: bool) instead of changing the default? |
||
| def _create_bounds_for_load_factor(self): | ||
| # TODO: Add Variable load_factor for better evaluation? | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Unify and harden pre-release detection; drive GH Release flag from this output
Follow-up (in “Create GitHub Release” step): set
prerelease: ${{ steps.prerelease.outputs.is_prerelease == 'true' }}so both places use the same signal.📝 Committable suggestion
🤖 Prompt for AI Agents