Repository navigation
fix(photonics2d): honor lambda1/lambda2 passed per call - #275
Merged
Merged
Conversation
SoheylM
reviewed
Sep 2, 2026
SoheylM
left a comment
Contributor
There was a problem hiding this comment.
The wavelength fix looks correct. Recomputing omega1 and omega2 from the merged per-call configuration covers simulate, optimize, and render. The new regression tests also pass locally: 15 passed.
The remaining actionable blocker is formatting: GitHub Actions’ Ruff 0.16.3 reports that tests/test_photonics2d.py would be reformatted. Please run ruff format tests/test_photonics2d.py and push the result.
The Linux 3.10 failure appears unrelated to this change: two Podman container tests fail because newuidmap is unavailable. A new push will rerun CI against the current base. Once formatting passes and the required checks are green, I’m happy to approve.
mkeeler43
force-pushed
the
fix/photonics-per-call-wavelengths
branch
from
September 3, 2026 16:41
85a2760 to
48f7b15
Compare
mkeeler43
force-pushed
the
fix/photonics-per-call-wavelengths
branch
from
September 3, 2026 16:55
48f7b15 to
b60e94e
Compare
The solver reads omega1/omega2, which v0 only computes in __init__, so wavelengths passed in config were ignored. v1 now recomputes them in _setup_simulation. Fixes #274.
mkeeler43
force-pushed
the
fix/photonics-per-call-wavelengths
branch
from
September 3, 2026 17:09
b60e94e to
acf8d82
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #274.
simulate(design, config={"lambda1": ..., "lambda2": ...})ignored the wavelengths. The solver readsself.omega1/self.omega2, which v0 only computes in__init__; all other conditions are read from the dict_setup_simulationreturns, which is whyconfig=works for them.v1 now overrides
_setup_simulationto recompute both frequencies from the merged conditions.simulateandoptimizeboth call it.v0 is unchanged so that published v0 results stay reproducible.
Verified
Simulating the first five test-set designs under their own conditions, with one problem instance and the conditions passed per call:
The same design at the default wavelengths still gives 0.2199, so the fix changes the result only when the wavelengths differ.
Tests
test_simulate_honors_wavelengths_passed_per_call: wavelengths passed per call give the same result as wavelengths passed to the constructor. Fails on main.test_dataset_row_reproduces_its_own_objective: simulating a dataset design under its own conditions returns the stored objective. This would be worth adding for the other problems too;beams2dpasses it to within 1e-5.