Improve speed of Box derivative computation when in GeometryGroup #2867
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| print("Overlaps (deg):") | ||
| print(f"Box vs. PolySlab Adjoint: {box_polyslab_overlap_deg}") | ||
| print(f"Box vs. PolySlab Finite Differnece: {fd_overlap_deg}") |
There was a problem hiding this comment.
syntax: Typo: 'Differnece' should be 'Difference'
| print(f"Box vs. PolySlab Finite Differnece: {fd_overlap_deg}") | |
| print(f"Box vs. PolySlab Finite Difference: {fd_overlap_deg}") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_components/autograd/numerical/test_autograd_box_polyslab_numerical.py
Line: 395:395
Comment:
**syntax:** Typo: 'Differnece' should be 'Difference'
```suggestion
print(f"Box vs. PolySlab Finite Difference: {fd_overlap_deg}")
```
How can I resolve this? If you propose a fix, please make it concise.
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
tidy3d/components/geometry/base.pyLines 2494-2502 2494
2495 is_2d_map.append(np.isclose(extents[axis_idx], 0.0))
2496
2497 if np.all(is_2d_map):
! 2498 return 0.0
2499
2500 is_2d = np.any(is_2d_map)
2501
2502 sim_bounds_normal, sim_bounds_perp = self.pop_axis( |
There was a problem hiding this comment.
+99 / -346 love to see it 😄
Caught some minor issues but overall looks great, thanks @groberts-flex
| # permittivity data | ||
| eps_xyz = [derivative_info.eps_data[f"eps_{dim}{dim}"] for dim in "xyz"] | ||
| if min_max_index == 0: | ||
| if coord_normal_face <= derivative_info.simulation_bounds[0][axis_normal]: |
There was a problem hiding this comment.
I remember we discussed this at some point but I need a refresher 😆 So now we zero gradients for faces that sit at the simulation boundaries, right? It's an edge case, but I feel like these should be strict comparisons?
| integration_bounds_perp = ( | ||
| np.maximum(bounds_perp[nonzero_dim][0], sim_bounds_perp[nonzero_dim][0]), | ||
| np.minimum(bounds_perp[nonzero_dim][1], sim_bounds_perp[nonzero_dim][1]), |
There was a problem hiding this comment.
It might be good to check here if the resulting interval is valid. If a box lies completely outside of the domain I think we'd end up with lower > upper here? And then spacing_to_grid_points will create invalid data for the interpolators I think.
Connect box face derivatives to
evaluate_gradient_at_pointsinDerivativeInfoso that interpolators can be shared when using inside aGeometryGroup.Greptile Overview
Updated On: 2025-10-06 19:50:07 UTC
Summary
This PR optimizes the performance of Box geometry derivative computation when used inside a GeometryGroup by refactoring the implementation to leverage the existing `evaluate_gradient_at_points` infrastructure in `DerivativeInfo`. The key changes include:Major refactoring of Box derivative computation: The
_derivative_facemethod intidy3d/components/geometry/base.pywas completely rewritten, removing 380+ lines of complex field integration code and replacing it with a simpler grid-based approach that builds spatial grids and evaluates gradients at specific points.Performance optimization through interpolator sharing: By connecting to the
evaluate_gradient_at_pointsmethod, the new implementation allows interpolators to be shared when multiple Box geometries are processed together in a GeometryGroup, significantly improving computational efficiency.Code simplification: The refactoring eliminates the need for PEC-specific handling, complex epsilon data management, and manual interpolation logic, making the codebase more maintainable while leveraging existing gradient evaluation frameworks.
Comprehensive testing: A new test file was added to validate that the optimized Box gradient implementation produces mathematically equivalent results to the established PolySlab approach across different dimensional configurations (2D and 3D cases).
Test cleanup: Obsolete Box-specific test functions were removed since the functionality is now covered by the broader gradient evaluation system.
This change fits into the broader Tidy3D architecture by unifying Box derivative computation with the existing
DerivativeInfoframework, enabling better resource sharing and performance optimization when working with multiple geometries in electromagnetic simulations.Changed Files
CHANGELOG.mdtidy3d/components/geometry/base.py_derivative_facemethod to use grid-based approach withevaluate_gradient_at_pointstests/test_components/autograd/numerical/test_autograd_box_polyslab_numerical.pytests/test_components/test_geometry.pyConfidence score: 4/5
Sequence Diagram
sequenceDiagram participant User participant Geometry participant DerivativeInfo participant Interpolators participant GradientEvaluator User->>Geometry: "_compute_derivatives(derivative_info)" Geometry->>DerivativeInfo: "create_interpolators(dtype=GRADIENT_DTYPE_FLOAT)" DerivativeInfo->>Interpolators: "create field data interpolators" Interpolators-->>DerivativeInfo: "interpolators" DerivativeInfo-->>Geometry: "interpolators" loop For each derivative path Geometry->>Geometry: "_derivative_faces(derivative_info)" loop For min/max faces and axes Geometry->>Geometry: "_derivative_face(min_max_index, axis_normal, derivative_info)" Geometry->>DerivativeInfo: "evaluate_gradient_at_points(spatial_coords, normals, perps1, perps2, interpolators)" DerivativeInfo->>GradientEvaluator: "interpolate fields at points" GradientEvaluator->>Interpolators: "get field values" Interpolators-->>GradientEvaluator: "field values" GradientEvaluator-->>DerivativeInfo: "gradient values" DerivativeInfo-->>Geometry: "gradient_at_points" Geometry->>Geometry: "sum(integration_weight * gradient_at_points)" Geometry-->>Geometry: "vjp_face" end Geometry->>Geometry: "_derivatives_center_size(vjps_faces)" Geometry-->>Geometry: "vjps_center_size" end Geometry-->>User: "derivative_map"