Internal cleanup: dedup, de-package solver benchmark, lazy matplotlib - #43
Merged
Merged
Conversation
These functions (_resolve_max_workers, _available_memory_bytes, _multiprocessing_start_method, _worker_initializer, _current_process_memory_bytes, _calibrate_auto_max_workers_from_result, _parallel_worker_execute) were verbatim copies of the canonical versions in batch.py. Nothing imported them from this module, and they referenced names never imported here (sys, ctypes, os, MaxWorkers, _run_payload, SolverProfiler), so they would have raised NameError if called. One copy also carried the macOS fork start-method that batch.py had already fixed to "spawn". No behaviour change: the record-conversion helpers that other modules actually import are untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
solver_benchmark.py is a developer tool, not a public API: it pulled matplotlib and tqdm into `import grax` and had a main() with no console-script entry and no tests. Move it to tools/solver_benchmark/ (alongside the other dev tools), drop its names from grax.__all__, and delete the lazy __getattr__ shim that only existed to import it. Imports switched from package-relative to absolute `grax.*`. Fixes carried in the move: - Type BenchmarkCase.grating_factory as Callable[[BenchmarkPreset], BaseGrating] and drop the `# type: ignore`; annotate run_solver_benchmark's energies_ev. - Serial path: run the timed repeats profiler-free and take a single extra untimed profiling pass, so profiler overhead no longer contaminates the samples and the kept profile is deterministic (was rebuilt every repeat and overwritten). - benchmark_energies() clamps/truncates to 100 points instead of raising. - Lift the "10 serial / 100 multiprocessing" default-count branch out of the call argument into a named local; memoize default_cases() with lru_cache. - export_benchmark(): 4-space indentation, grouped filtering, no over-long lines. - README documents that the serial (fixed-angle) and multiprocessing (cff monochromator sweep) modes measure deliberately different things. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_refractive_index_row, _build_material_code_grid, and _build_refractive_index_grid each carried their own copy of the same non-multilayer layer walk: seed the bottom interface, then for every layer accumulate thickness, evaluate the rough upper interface, build a z-mask and write a payload, advance. Only the payload (refractive index vs material code) and the 1-D/2-D masking differed. Add BaseGrating._iter_rough_layer_interfaces(), a generator yielding (material_name, lower_interface, upper_interface) bottom-up with a final (None, top_interface, None) sentinel for the incident medium, and have the three methods consume it. The _rough_interface call sequence and interface indices are unchanged, so output is bit-identical: a before/after capture of the three grids, _refractive_index_row over 15 z-levels, and end-to-end RCWA/Nevière efficiencies for laminar / blazed / blazed-multilayer gratings (smooth and random-interface rough) matches exactly across all 72 arrays. Full unit suite and smoke tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Six shipped modules did `import matplotlib.pyplot as plt` at module scope, so every `import grax` (including the re-import each spawned batch worker performs) paid the full pyplot + backend cost -- ~160 ms, about a third of import time -- even for headless library and batch use. afm_preprocessing.py already imports pyplot lazily inside its plotting functions; this brings the rest in line. Each plotting function/method now imports pyplot locally; the four modules that use `plt` only in type annotations also carry a `TYPE_CHECKING` import so those annotations still resolve for type checkers and linters. matplotlib.ticker in parameter_sweep.py gets the same treatment. `import grax` no longer pulls in matplotlib.pyplot (verified via sys.modules); plotting behaviour is unchanged. One white-box test that reached into `grax.simulation.batch.plt` now patches `matplotlib.pyplot` directly (same singleton the lazy import resolves to). Full unit + smoke suites pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Review-driven cleanup pass. No public API or numerical behaviour changes.
Changes
simulation/serialization.py— 168 lines of copy-pasted_resolve_max_workers/_multiprocessing_start_method/_worker_initializer/ etc. that nothing imported, referenced undefined names, and carried the stale macOSforkstart methodbatch.pyhad already fixed.solver_benchmark.pyout of the package intotools/solver_benchmark/— it's a developer tool, not public API, and pulled matplotlib + tqdm intoimport grax. Dropped fromgrax.__all__and the lazy__getattr__. Fixes carried in the move: serial path no longer 4×-times each configuration, timed repeats run profiler-free,benchmark_energiesclamps instead of raising,Callabletyping, README documents the serial-vs-multiprocessing difference.gratings.py—_refractive_index_row,_build_material_code_grid,_build_refractive_index_gridshared their layer walk into_iter_rough_layer_interfaces. Verified bit-identical: 72-array before/after capture (the three grids,_refractive_index_rowover 15 z-levels, end-to-end RCWA/Nevière efficiencies for laminar/blazed/multilayer × smooth/rough) matches exactly.matplotlib.pyplotlazily across 6 modules —import graxdropped from ~0.51s to ~0.36s and no longer loads pyplot. Plotting behaviour unchanged;TYPE_CHECKINGblocks keep annotations resolving.Verification
tests/unit+tests/smoke: 481 passed, 5 skipped (skips are RETICOLO/Octave-gated, pre-existing).developon every touched file.src/change: +103 / −549 lines.🤖 Generated with Claude Code