Skip to content

PR 1/7: CI/CD infrastructure for Pyomo testing#6

Open
bernalde wants to merge 5 commits intomainfrom
pr/ci-cd-pyomo
Open

PR 1/7: CI/CD infrastructure for Pyomo testing#6
bernalde wants to merge 5 commits intomainfrom
pr/ci-cd-pyomo

Conversation

@bernalde
Copy link
Member

@bernalde bernalde commented Mar 3, 2026

Summary

Updates CI workflows to handle Pyomo as an optional dependency. Tests are split into core (always run) and pyomo (only when Pyomo is installed). Adds @pytest.mark.pyomo marker registration.

Also includes a bugfix for RampInterpolator (fixes LyoHUB/LyoPRONTO#17).

PR 1 of 7 in the Pyomo integration series. See the full plan in PR #5.

Changes

CI/CD

  • .github/workflows/pr-tests.yml — Add Pyomo-aware test splits (separate test-scipy and test-pyomo jobs); install Pyomo deps explicitly (not via [optimization] extra, which is defined in PR PR 2/7: Pyomo optional dependencies and module structure #7)
  • .github/workflows/slow-tests.yml — Pyomo slow test support with explicit dep install
  • .github/workflows/tests.yml — Full test workflow with Pyomo; remove dev-pyomo branch trigger (replaced by PR chain)
  • pytest.ini — Add serial, pyomo, notebook markers

Bug Fix (LyoHUB#17)

  • lyopronto/functions.py — Fix RampInterpolator.__init__(): (1) prevent double-counting of dt_setpt[0] in the no-init path, (2) clamp negative holdtime to 0 instead of silently producing incorrect schedules
  • tests/test_functions.py — Add two regression tests for the RampInterpolator fix; fix assertion comments ("monotonically non-decreasing" to match >= 0 check)

PR Chain

# PR Status
0 Sync upstream (#5) Merged
1 CI/CD for Pyomo + RampInterpolator fix (this PR)
2 Pyomo dependencies Pending
3 Utils & single-step Pending
4 Multi-period model Pending
5 Optimizer functions Pending
6 Benchmarks Pending
7 Docs & examples Pending

Testing

  • All 85 existing tests pass (including 15 RampInterpolator tests)
  • Two new regression tests added for the RampInterpolator fix
  • CI workflows install Pyomo deps explicitly via pip install pyomo idaes-pse (the [optimization] extra is not defined until PR PR 2/7: Pyomo optional dependencies and module structure #7)

This PR adds the CI/CD infrastructure to support Pyomo tests alongside scipy tests.

Key changes:
- .github/workflows/tests.yml: Add separate test-scipy and test-pyomo jobs
- .github/workflows/pr-tests.yml: Add Pyomo tests with continue-on-error
- .github/workflows/slow-tests.yml: Add include_pyomo option
- pytest.ini: Add 'pyomo' and 'notebook' markers

The Pyomo tests:
- Install pyomo and idaes-pse from [optimization] extras
- Install IPOPT solver via 'idaes get-extensions --extra petsc'
- Run with continue-on-error: true (don't block PRs/merges)
- Use marker: @pytest.mark.pyomo

This enables running Pyomo tests independently while not blocking
scipy-based workflows.
Fix two bugs in RampInterpolator.__init__() (LyoHUB#17):

1. No-init path double-counting: When rampspec lacks an 'init' key,
   dt_setpt[0] was consumed both during initialization (times starts
   with [0, dt_setpt[0]]) and in the loop at i=1 (via i-1=0 index).
   Fix: use dt_idx=i (not i-1) in the no-init path so the loop starts
   consuming at dt_setpt[1].

2. Negative holdtime: When ramp time exceeds total stage time, holdtime
   goes negative, producing non-monotonic cumulative times. The warning
   condition was also wrong (ramptime > holdtime instead of holdtime < 0).
   Fix: clamp holdtime to 0 and improve the warning message.

Add regression tests for both bugs.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates CI workflows to support running Pyomo-related tests separately from the SciPy/core test suite, and registers new pytest markers to enable selective execution. Despite the PR description indicating “CI/CD only”, it also includes a functional fix and regression tests for RampInterpolator.

Changes:

  • Split CI test execution into SciPy/core vs Pyomo-marked tests across PR, main, and manual slow-test workflows.
  • Register additional pytest markers (serial, pyomo, notebook) and enable --dist loadgroup for xdist.
  • Fix RampInterpolator time accounting (no-init multi-setpoint double-count) and clamp negative hold times; add regression tests for both.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
.github/workflows/pr-tests.yml Adds separate optional Pyomo job and excludes pyomo tests from SciPy PR runs.
.github/workflows/tests.yml Splits main-branch CI into SciPy vs optional Pyomo jobs with separate coverage uploads.
.github/workflows/slow-tests.yml Adds a manual toggle to include/exclude Pyomo tests and IPOPT setup.
pytest.ini Registers new markers and adds --dist loadgroup to default pytest options.
lyopronto/functions.py Fixes RampInterpolator scheduling logic and clamps negative hold times with a warning.
tests/test_functions.py Adds regression tests for the RampInterpolator fixes.

- Replace pip install .[optimization] with explicit pip install pyomo
  idaes-pse in all three CI workflows (pr-tests.yml, tests.yml,
  slow-tests.yml). The optimization extra is not defined until PR 2,
  so the Pyomo jobs need to install deps explicitly for now.
- Fix misleading 'strictly monotonic' comment in test assertions to
  say 'monotonically non-decreasing' (holdtime can be clamped to 0).
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

pytest -m 'pyomo' exits with code 5 when no pyomo-marked tests exist.
This is expected until PR #8 adds the first pyomo tests. Treat exit
code 5 as success so the Pyomo Tests job passes gracefully.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

…rker

- Add 'if: draft == false' to test-pyomo job in pr-tests.yml so draft
  PRs get fast feedback without installing Pyomo/IDAES/IPOPT.
- Update serial marker description to clarify it requires manual
  enforcement via 'pytest -m serial -n 0'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RampInterpolator: potential double-counting of first stage duration in no-init path

2 participants