Change EME validation to only depend on transverse grids (FXC-3746)

[caseyflex]

Previously, EME validated based on the total number of grid points in the simulation. It is better to only validate based on the transverse grid points in the mode solver monitors. The max number of EME cells is handled elsewhere.

I'm not too sure about the numbers, the max here seems pretty large but it's consistent with the current unit tests. Let me know if you have suggestions for more reasonable max number of transverse grid points which would still handle applications of interest.

Greptile Overview

Updated On: 2025-11-05 20:17:05 UTC

Greptile Summary

Refactored EME simulation validation to check transverse grid points in mode solver monitors rather than total simulation grid points.

Key Changes:

• Removed MAX_GRID_CELLS constant and validation check from _validate_size() method
• Added MAX_MODE_NUM_CELLS constant (2e8)
• Enhanced _validate_modes_size() to raise SetupError when transverse grid points exceed MAX_MODE_NUM_CELLS
• Updated validation logic to check each mode monitor's transverse computational cells using discretize_monitor()
• Added changelog entry explaining the change

Impact:
This change makes the validation more targeted and appropriate for EME simulations, where the relevant constraint is the transverse grid resolution used for mode solving, not the total 3D simulation size.

Confidence Score: 5/5

• This PR is safe to merge - it improves validation logic appropriately for EME simulations
• The change is well-targeted and correctly implements the intended validation improvement. The old validation checked total simulation grid cells which was overly restrictive for EME. The new validation properly checks transverse grid points in mode monitors, which is the actual constraint for EME mode solving. The constant value (2e8) is consistent with existing tests and appropriate for the use case.
• No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
CHANGELOG.md	5/5	Added changelog entry for EME validation change - correctly categorized under Fixed section
tidy3d/components/eme/simulation.py	5/5	Refactored EME validation to check transverse grid points only instead of total simulation cells - implementation looks correct

Sequence Diagram

sequenceDiagram
    participant User
    participant EMESimulation
    participant validate_pre_upload
    participant _validate_size
    participant _validate_modes_size
    participant warn_mode_size
    
    User->>EMESimulation: Create simulation
    EMESimulation->>validate_pre_upload: Validate before upload
    validate_pre_upload->>_validate_size: Check frequency limits
    _validate_size-->>validate_pre_upload: OK
    validate_pre_upload->>_validate_modes_size: Check mode monitor sizes
    _validate_modes_size->>warn_mode_size: Check each monitor
    warn_mode_size->>EMESimulation: discretize_monitor(monitor)
    EMESimulation-->>warn_mode_size: Grid with num_cells
    warn_mode_size->>warn_mode_size: Calculate num_cells = prod(num_cells)
    alt num_cells > MAX_MODE_NUM_CELLS
        warn_mode_size-->>_validate_modes_size: Raise SetupError
        _validate_modes_size-->>validate_pre_upload: Error
        validate_pre_upload-->>User: Validation failed
    else num_cells > WARN_MODE_NUM_CELLS
        warn_mode_size-->>_validate_modes_size: Log warning
    else
        warn_mode_size-->>_validate_modes_size: OK
    end
    _validate_modes_size-->>validate_pre_upload: OK
    validate_pre_upload-->>User: Validation passed

Loading

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

Diff Coverage

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

• tidy3d/components/eme/simulation.py (100%)

Summary

• Total: 3 lines
• Missing: 0 lines
• Coverage: 100%

[caseyflex]

ok, I made sure the tests were actually testing the new limit, and I was able to reduce it to a more reasonable value. Still worth checking that it is large enough -- it is only a bit after this point that the mode solver monitor storage starts failing validation

[momchil-flex]

Makes sense! Field monitors that span the propagation direction will still be prohibited if too large right?

[caseyflex]

Makes sense! Field monitors that span the propagation direction will still be prohibited if too large right?

Yes on the basis of storage limits, which may be large compared to practical field computation limits, so maybe we want additional validation, not sure. Those are also included in FC cost

maybe we wait and see if @FilipeFcp runs into anything

[FilipeFcp]

Thanks @caseyflex
I tested this with the model I was originally trying to run.

It makes much more sense to me now that the validation depends on the mode planes rather than the waveguide length.

Regarding the EMEFieldMonitor, it seems to be validating correctly (with a maximum of 50 GB).

However, it appears there is no validation that considers the grid points in the mode plane and the number of grid cells. I’m running this model with 100 cells, close to the maximum limit, and I think it might run for a very long time.

Is this intentional, or do you think we should also include a number_of_cells × grid_points validation?

[caseyflex]

Great @FilipeFcp

I think let’s validate num transverse grid and num EME cells separately, as they are used independently in the calculation. I think if you are running such a big simulation, you would expect it to be a bit slow, and that’s probably better than not allowing it!

[caseyflex]

@FilipeFcp the estimates on your model seem not so bad. Is there anything strange that would make it take a long time? Like a large field monitor?