Skip to content

fix: align power electronics constraints with API - #272

Merged
SoheylM merged 3 commits into
mainfrom
fix/243-power-electronics-constraints
Sep 16, 2026
Merged

SoheylM merged 3 commits into
mainfrom
fix/243-power-electronics-constraints

Conversation

@SoheylM

@SoheylM SoheylM commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Description

Align the Power Electronics v0 problem with the constraint API described in the current documentation.

  • Add THEORY error constraints for positive passive-component values, a fractional duty cycle, and binary switch levels.
  • Add an IMPLEMENTATION error constraint for the PWL timing range imposed by the 10 ns transition time and 5 us switching period.
  • Preserve the existing v0 design space as the supported benchmark and dataset domain.
  • Warn and return non-finite objectives when ngspice output is invalid, allowing batch generation to continue.
  • Remove switch-model parameters ignored by ngspice.
  • Preserve dataclass fields with init=False when checking constraints against an existing problem configuration.
  • Correct the constraint-category example in the developer documentation and document the Power Electronics constraints.

Part of #243 (Power Electronics).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

Screenshots

Not applicable.

Validation

  • pre-commit run --all-files
  • ruff check .
  • ruff format --check .
  • Focused tests: 35 passed
  • Broader tests excluding tests/test_problem_implementations.py: 78 passed, 9 skipped
  • mypy passes for the affected source files
  • A valid ngspice simulation returns finite objectives
  • An invalid topology emits InvalidNgSpiceOutputWarning and returns NaNs without stopping the process

Local environment notes:

  • The repository-wide mypy command currently reports pre-existing NumPy typing failures across 53 files.
  • tests/test_problem_implementations.py cannot be collected locally because pytest-subtests is not installed in the project test environment.

Checklist

  • I have run the pre-commit checks with pre-commit run --all-files
  • I have run ruff check . and verified formatting with ruff format --check .
  • I have run mypy .
  • I have commented my code where the behavior is not self-explanatory
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove the fix is effective
  • New and existing unit tests pass locally with my changes

The unchecked warning item is intentional: invalid ngspice output now raises a dedicated warning while preserving NaNs for batch processing. The unchecked test and mypy items are explained in the validation notes above.

Reviewer Checklist

  • The content of this PR brings value to the community. It is not too specific to a particular use case.
  • The tests and checks pass.
  • The documentation is updated.
  • The code is understandable and appropriately commented.
  • There is no merge conflict.
  • The changes do not break existing benchmark results.
  • For bug fixes, the fix is robust.

@SoheylM
SoheylM requested a review from g-braeunlich September 2, 2026 14:19

@g-braeunlich g-braeunlich left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only some very minor suggestions!

Comment thread engibench/problems/power_electronics/utils/process_log_file.py Outdated
Comment thread tests/test_power_electronics.py Outdated
Comment thread tests/test_power_electronics.py Outdated
Comment thread tests/test_power_electronics.py Outdated
@SoheylM

SoheylM commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@g-braeunlich Thanks for the review. I addressed the comments and CI is green. Are you happy for us to merge?

@SoheylM
SoheylM merged commit 0173289 into main Sep 16, 2026
20 of 21 checks passed
@SoheylM
SoheylM deleted the fix/243-power-electronics-constraints branch September 16, 2026 14:53
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.

2 participants