Skip to content

Conversation

@caseyflex
Copy link
Contributor

@caseyflex caseyflex commented Sep 30, 2025

2D materials are subdivided when they have inhomogeneous substrates. The new structures were inheriting the name from the old structure, giving an error due to name reuse. Also, this validator was not run for EME simulations.

This PR:

  • fixes the subdivision and adds a test
  • checks sim._finalized for any AbstractYeeGridSimulation in validate_pre_upload (which must now be called by subclasses)

Greptile Overview

Updated On: 2025-09-30 20:38:03 UTC

Summary

This PR fixes a critical bug affecting named 2D materials with inhomogeneous substrates. When Tidy3D encounters 2D materials on inhomogeneous substrates, it automatically subdivides them into volumetric equivalents for simulation accuracy. However, the subdivision process was inheriting the original structure's name for all subdivided pieces, causing validation errors due to duplicate names within the simulation.

The fix implements two key changes: First, it modifies the subdivision logic in simulation.py to append unique suffixes (_SUBDIVIDED[{i}]) to each subdivided structure's name, preventing naming conflicts. Second, it restructures the validation hierarchy by moving the _validate_finalized call to the base AbstractYeeGridSimulation.validate_pre_upload method and ensuring EME simulations properly call the parent validation through super().validate_pre_upload().

These changes ensure that all simulation types (FDTD, EME, etc.) that inherit from AbstractYeeGridSimulation consistently handle 2D material subdivision validation, maintaining the integrity of the simulation setup process while preventing solver errors.

Important Files Changed

Changed Files
Filename Score Overview
CHANGELOG.md 5/5 Added changelog entry documenting the fix for named 2D materials with inhomogeneous substrates
tidy3d/components/simulation.py 4/5 Fixed subdivision naming conflicts by adding unique suffixes and moved validation to base class
tests/test_components/test_simulation.py 4/5 Enhanced test coverage by adding named structure and ensuring finalization triggers validation
tidy3d/components/eme/simulation.py 5/5 Added missing super().validate_pre_upload() call to inherit critical parent class validations

Confidence score: 4/5

  • This PR is safe to merge with low risk as it addresses a specific bug with a targeted fix
  • Score reflects solid implementation and thorough testing, though the subdivision logic is moderately complex
  • Pay close attention to tidy3d/components/simulation.py for the subdivision naming logic and validation restructuring

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/components/eme/simulation.py (100%)
  • tidy3d/components/mode/simulation.py (100%)
  • tidy3d/components/simulation.py (91.7%): Missing lines 2160,2181,2226-2227

Summary

  • Total: 50 lines
  • Missing: 4 lines
  • Coverage: 92%

tidy3d/components/simulation.py

  2156             length = obj.frame.length
  2157             if direction == "+":
  2158                 span_inds[axis][1] += length - 1
  2159             else:
! 2160                 span_inds[axis][0] -= length - 1
  2161         else:
  2162             axis = obj.size.index(0.0)
  2163 
  2164         box_bounds = [

  2177         else:
  2178             if direction == "-":
  2179                 del surfaces[2 * axis + 1]
  2180             else:
! 2181                 del surfaces[2 * axis]
  2182 
  2183         structure = Structure(
  2184             geometry=GeometryGroup(
  2185                 geometries=surfaces,

  2222         """Validate that after adding pec frames simulation setup is still valid."""
  2223 
  2224         try:
  2225             _ = self._finalized
! 2226         except Exception:
! 2227             log.error(
  2228                 "Simulation fails after requested mode source PEC frames are added. "
  2229                 "Please inspect '._finalized'."
  2230             )

Copy link
Contributor

@dbochkov-flexcompute dbochkov-flexcompute left a comment

Choose a reason for hiding this comment

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

looks good! I wasn't sure initially whether super().validate_pre_upload() should be in the same begin_capture/end_capture, and whether _validate_finalized would produce duplicated set of warnings, but it seems to be working ok as it is

Copy link
Contributor

@dmarek-flex dmarek-flex left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@caseyflex caseyflex force-pushed the casey/2dmaterialvalidators branch from 3d1bc0e to e010091 Compare October 6, 2025 21:10
@caseyflex caseyflex enabled auto-merge October 6, 2025 21:11
@caseyflex caseyflex added this pull request to the merge queue Oct 6, 2025
Merged via the queue into develop with commit 7889fe6 Oct 6, 2025
22 checks passed
@caseyflex caseyflex deleted the casey/2dmaterialvalidators branch October 6, 2025 22:18
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.

4 participants