Skip to content

fix(photonics2d): honor lambda1/lambda2 passed per call - #275

Merged
mkeeler43 merged 1 commit into
mainfrom
fix/photonics-per-call-wavelengths
Sep 9, 2026
Merged

mkeeler43 merged 1 commit into
mainfrom
fix/photonics-per-call-wavelengths

Conversation

@mkeeler43

@mkeeler43 mkeeler43 commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #274.

simulate(design, config={"lambda1": ..., "lambda2": ...}) ignored the wavelengths. The solver reads self.omega1 / self.omega2, which v0 only computes in __init__; all other conditions are read from the dict _setup_simulation returns, which is why config= works for them.

v1 now overrides _setup_simulation to recompute both frequencies from the merged conditions. simulate and optimize both 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:

row stored objective before after
0 4.7460 0.2199 4.7460
1 5.7242 1.0781 5.7242
2 6.0318 0.3333 6.0318
3 5.7660 1.9619 5.7660
4 5.9565 1.3911 5.9565

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; beams2d passes it to within 1e-5.

@SoheylM SoheylM left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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
mkeeler43 force-pushed the fix/photonics-per-call-wavelengths branch from 85a2760 to 48f7b15 Compare September 3, 2026 16:41
@mkeeler43 mkeeler43 changed the title fix(photonics2d): honour lambda1/lambda2 passed per call fix(photonics2d): honor lambda1/lambda2 passed per call Sep 3, 2026
@mkeeler43
mkeeler43 force-pushed the fix/photonics-per-call-wavelengths branch from 48f7b15 to b60e94e Compare September 3, 2026 16:55
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
mkeeler43 force-pushed the fix/photonics-per-call-wavelengths branch from b60e94e to acf8d82 Compare September 3, 2026 17:09
@mkeeler43
mkeeler43 merged commit dbd753b into main Sep 9, 2026
15 checks passed
@mkeeler43
mkeeler43 deleted the fix/photonics-per-call-wavelengths branch September 9, 2026 15:12
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.

photonics2d ignores lambda1/lambda2 passed to simulate() and optimize()

3 participants