feat: frequency-dependent material dispersion and API refactor#143
Open
benvial wants to merge 19 commits into
Open
feat: frequency-dependent material dispersion and API refactor#143benvial wants to merge 19 commits into
benvial wants to merge 19 commits into
Conversation
This was referenced May 21, 2026
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive refactor of the material property system, transitioning to a frequency-aware resolution model that supports dispersion (Sellmeier and Lorentzian) and anisotropic tensors. It introduces a PDK overlay system and updates MEEP and Palace simulation logic, including support for shaped dielectric volumes. The review feedback identifies several regressions where the deprecated type field is still being passed to MaterialProperties, redundant logic in material resolution and configuration generation, and debug leftovers in the notebooks.
Comment on lines
+70
to
+71
| mat_type = entry.get("type", "dielectric") | ||
| props_kwargs: dict = {"type": mat_type} |
There was a problem hiding this comment.
The MaterialProperties model no longer contains a type field after the refactor. Passing it in props_kwargs is incorrect and may cause issues if strict validation is enabled elsewhere, or it is simply dead code.
Suggested change
| mat_type = entry.get("type", "dielectric") | |
| props_kwargs: dict = {"type": mat_type} | |
| props_kwargs: dict = {} |
| @@ -527,8 +543,8 @@ def _material_overrides(self) -> dict[str, Any]: | |||
| for name, val in self._resolved_materials().items(): | |||
| overrides[name] = MaterialProperties( | |||
| type="dielectric", | |||
There was a problem hiding this comment.
The MaterialProperties class no longer accepts a type argument. This will likely cause a ValidationError or unexpected behavior during material override resolution.
overrides[name] = MaterialProperties(| "print(sim.validate_config())" | ||
| "print(sim.validate_config())\n", | ||
| "\n", | ||
| "xsxs" |
There was a problem hiding this comment.
This looks like a typo or debug leftover that should be removed.
|
|
||
| print(sim.validate_config()) | ||
|
|
||
| xsxs |
There was a problem hiding this comment.
This looks like a typo or debug leftover that should be removed.
| "# Static PNG\n", | ||
| "sim.plot_mesh(show_groups=[\"metal\", \"P\"])" | ||
| "sim.plot_mesh(show_groups=[\"metal\", \"P\"])\n", | ||
| "xsx" |
There was a problem hiding this comment.
This looks like a typo or debug leftover that should be removed.
| cond = props.conductivity | ||
| if isinstance(cond, list): | ||
| cond = cond[0] | ||
| if cond is not None and cond >= 1e4: |
There was a problem hiding this comment.
Use the constant ResolvedMaterial.CONDUCTIVITY_THRESHOLD instead of the hardcoded value 1e4 to maintain consistency with the material resolution logic.
Suggested change
| if cond is not None and cond >= 1e4: | |
| if cond is not None and cond >= ResolvedMaterial.CONDUCTIVITY_THRESHOLD: |
| if isinstance(cond, list): | ||
| cond = cond[0] | ||
| if cond is not None and cond >= 1e4: | ||
| base.validity_note = "conductive material (no permittivity needed)" |
There was a problem hiding this comment.
Use the constant ResolvedMaterial.CONDUCTIVITY_THRESHOLD instead of the hardcoded value 1e4 for consistency.
Suggested change
| base.validity_note = "conductive material (no permittivity needed)" | |
| if cond is not None and cond >= ResolvedMaterial.CONDUCTIVITY_THRESHOLD: |
| resolved[name] = dict(props) | ||
| continue | ||
|
|
||
| evaluated = resolve_material_at_wavelength(name, wavelength_um) |
There was a problem hiding this comment.
This call to resolve_material_at_wavelength is redundant as the result was already computed and checked in the preceding lines (50-53).
| } | ||
|
|
||
| if is_shaped_dielectric: | ||
| shaped_dielectric_names.add(layer_name) |
There was a problem hiding this comment.
This add operation is redundant because is_shaped_dielectric is only true if layer_name is already present in the shaped_dielectric_names set.
| @@ -136,6 +141,11 @@ def validate_frequency_range(self) -> Self: | |||
| def to_palace_config(self) -> dict: | |||
| """Convert to Palace JSON config format.""" | |||
| freq_step = (self.fmax - self.fmin) / max(1, self.num_points - 1) / 1e9 | |||
There was a problem hiding this comment.
This calculation of freq_step is redundant as it is overwritten immediately below in the if/else block.
Add dispersion model types (ValidityRange, SellmeierTerm, LorentzianTerm, DispersionModel) to the materials database. Each material entry can store multiple dispersion models covering different frequency regimes (e.g. SiO2: Sellmeier for optical, constant-ε for RF), each annotated with validity range and citation. Key changes: - MaterialProperties gains dispersion_models, conductivity_diagonal - evaluate_at_wavelength/frequency resolves the correct model - resolve_material_at_wavelength() with validity checking and warnings - should_enable_dispersion() auto-heuristic (Δn/n > 0.5%) - ResolvedMaterial carries evaluation metadata - MEEP materials.py uses wavelength-aware resolution - MaterialData config supports anisotropic + dispersive fields - LorentzianPoleConfig for susceptibility poles - FDTD.dispersion flag (auto/true/false) - Palace config_generator handles conductivity_diagonal - PDK overlay system (load_overlay, merge_overlay) - MATERIALS_DB updated with Sellmeier fits and validity ranges - Comprehensive tests for dispersion models and overlays
Conductors have no optical data; silently skip them instead of emitting spurious warnings. Add germanium (Ge) to MATERIALS_DB with Sellmeier fit from Barnes & Piltch 1979 (2.5-12 µm).
Replace Unicode symbols with ASCII equivalents for cp1252 compatibility. Add missing docstrings for interrogate coverage. Fix line length violations. Fix ruff lint warnings in tests.
Add overlay parameter to resolve_material_at_wavelength() and should_enable_dispersion(). Add _resolve_with_overlay() helper implementing priority: user override > PDK overlay > built-in DB. Thread overlay through MEEP resolve_materials(). Add Simulation.load_pdk_overlay() and pass overlay in build_config().
Map ResolvedMaterial anisotropic fields to MaterialData: epsilon_diag, mu_diag, D_conductivity, D_conductivity_diag, epsilon_offdiag. Add loss_tangent_to_conductivity() converter (sigma = 2*pi*f*eps0*er*tand). Add _rotate_diagonal_tensor() for material_axes rotation. Add _resolved_to_material_data() as unified converter.
Implement sellmeier_to_lorentzian_poles() converter mapping Sellmeier terms to MEEP LorentzianSusceptibility poles (w0=1/sqrt(C), sigma=B*w0^2, gamma=0). Add lorentzian_to_meep_poles() for direct Lorentzian mapping. Reimplement resolve_materials_with_dispersion() with full dispersion flag logic: auto evaluates dn/n per material, true forces all dispersive, false forces constant-epsilon. Wire through Simulation.build_config() when solver.dispersion != 'false'.
Add resolve_palace_materials_at_frequency() to evaluate dispersion models at a target frequency, producing a materials dict for Palace config generation. Add frequency_hz parameter to generate_palace_config() and write_config(). When provided, material permittivity is evaluated from Sellmeier/Lorentzian models at that frequency instead of using static values. Implements the RFC's external frequency loop strategy for Palace (which lacks native epsilon(f) support).
Merge permittivity_diagonal → permittivity (float | list[float]), conductivity_diagonal → conductivity, loss_tangent_diagonal → loss_tangent. Add _as_list() and _is_tensor() helpers. Add scalar convenience properties. Remove pydantic ge/le constraints from union-typed fields. Update overlays, MEEP, Palace, and all tests. 541 tests pass, all pre-commit hooks pass.
Replace refractive_index + extinction_coeff with permittivity + loss_tangent as the sole solver-facing representation. Removed from ResolvedMaterial, MaterialProperties, DispersionModel, MaterialData, and all downstream consumers (MEEP, Palace, overlays, ports, script). - evaluate_at_wavelength() always sets permittivity directly - index_variation() computes deps/eps instead of dn/n - MaterialData uses epsilon_diag as primary field (no refractive_index) - build_materials() in script.py uses mp.Medium(epsilon=) directly - Removed MaterialProperties.optical() classmethod - Removed DispersionModel.refractive_index field - 540 tests pass, all 23 pre-commit hooks pass
…loat shorthand
Replace refractive-index-based Material(n, k) with permittivity-based
Material(permittivity, loss_tangent). Float shorthand like {"si": 12.0}
is interpreted as permittivity. Remove float-to-n**2 bridge in
_material_overrides. Update dispersion docstrings to deps/eps. Fix
pre-existing indentation bug in _cloud_fixtures.py.
…ter freq Add f parameter to set_driven(): sim.set_driven(f=50e9) sets fmin=fmax with num_points=1. Explicit fmin/fmax override f. Add DrivenConfig.center_frequency property. Remove frequency_hz from generate_palace_config/write_config; dispersion is now auto-evaluated at (fmin+fmax)/2. Fix c: 3e8 -> 299_792_458 everywhere. Remove stray 'xsx' in palace_microstrip notebook.
Auto-detect shaped dielectrics from layer_type=dielectric + thin patterned layers (<5um) with polygon geometry. Remove manual shaped_dielectrics parameter and layer.shaped field entirely. Photonic stacks (ubcpdk) get BOX preserved as SiO2 via has_patterned_dielectrics auto-detection. Silicon conductivity dropped when Sellmeier covers target frequency.
…e check Shaped dielectric detection now uses _detect_shaped_dielectric_layers(): a dielectric layer is shaped iff it has polygon geometry AND its (material, z-range) is NOT already covered by a stack.dielectrics box. Thickness threshold was fragile; box-coverage is structurally correct.
Silicon: Salzberg & Villa 3-term with correct B3=1.541/C3=1104^2 (n=3.478 at 1550nm, was 3.634 with wrong 3rd term). SiO2: Malitson 1965 full-precision coefficients, wider 0.21-6.7um range. Si3N4: Luke 2015 NIR 2-term model (meep: Si3N4_NIR), wider 0.31-5.5um. Sapphire: Malitson & Dodge 1972 3-term ordinary-axis (n_o=1.746 at 1550nm). Germanium: Icenogle 1979 with epsilon_inf=9.28 (meep: Ge).
…equency-aware behavior Material.type (conductor/dielectric/semiconductor) is frequency-independent but material behavior is not — Si is dielectric at optical, conductive at RF. Replace with ResolvedMaterial.behavior property that decides "conductive" vs "dielectric" at evaluation time using conductivity threshold (>=1e4 S/m) and dispersive model presence. BREAKING CHANGE: MaterialProperties.type field removed. Use resolved.behavior instead of material_is_conductor/dielectric.
…erials Conductive materials (Al, Cu, etc.) have no permittivity field — this is expected, not an error. Move behavior check before permittivity-is-None check in resolve_materials_with_dispersion, and skip warning in resolve_material_at_wavelength when behavior is conductive.
SiO2 validity range updated 3.71->6.7um, source string relaxed. Ge dispersion test moved to wl=3.0/bw=1.0 where variation exceeds threshold (new Icenogle model is much flatter at 4um than old model).
MEEP run_local() writes config/GDS/script, then executes run_meep.py locally via Python (with meep installed). Supports MPI via num_processes parameter. Returns SParameterResult parsed from CSV. Also updates silicon n@1550nm test bounds from 3.5-4.2 to 3.4-3.6 after correcting Sellmeier coefficients (n=3.478, was 4.034).
benvial
force-pushed
the
feat/frequency-dependent-materials
branch
from
b82a36f to
4a1dad4
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #143 +/- ##
==========================================
+ Coverage 55.55% 57.45% +1.89%
==========================================
Files 59 61 +2
Lines 6966 7650 +684
Branches 1270 1446 +176
==========================================
+ Hits 3870 4395 +525
- Misses 2697 2827 +130
- Partials 399 428 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Summary
dispersion=autothrough the MEEP build pipelineMaterial(permittivity, loss_tangent)replacesMaterial(n, k), removerefactive_index/extinction_coeffset_driven(f=...)single-freq shorthand with auto-dispersion at center frequencyshaped_dielectricsparamBreaking Changes
Material(n, k)→Material(permittivity, loss_tangent): float shorthand now interprets as permittivity, not refractive indexrefractive_indexandextinction_coeffremoved from material APIshaped_dielectricsparam removed (auto-detected now)Dependencies