-
Notifications
You must be signed in to change notification settings - Fork 69
feat(tidy3d): FXC-3296-autograd-support-for-anisotropic-medium-and-custom-anisotropic-medium #3080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
eb1d806 to
9a1d670
Compare
There was a problem hiding this 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 1 comment
9a1d670 to
493bef5
Compare
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
tidy3d/components/autograd/derivative_utils.pyLines 734-742 734 if the requested map is unavailable.
735 """
736 field_map = {"E": self.E_der_map, "D": self.D_der_map}.get(field_type)
737 if field_map is None:
! 738 raise ValueError("field type must be 'D' or 'E'.")
739
740 axis = axis.lower()
741 projected = dict(field_map)
742 if not field_map:Lines 743-751 743 return projected
744 for dim in "xyz":
745 key = f"E{dim}"
746 if key not in field_map:
! 747 continue
748 if dim != axis:
749 projected[key] = _zeros_like(field_map[key])
750 else:
751 projected[key] = field_map[key]tidy3d/components/medium.pyLines 6128-6136 6128 component_paths = [
6129 tuple(path[1:]) for path in derivative_info.paths if path and path[0] == component
6130 ]
6131 if not component_paths:
! 6132 return None
6133
6134 axis = component[0] # f.e. xx -> x
6135 projected_E = derivative_info.project_der_map_to_axis(axis, "E")
6136 projected_D = derivative_info.project_der_map_to_axis(axis, "D")Lines 6143-6151 6143
6144 components = self.components
6145 for field_path in derivative_info.paths:
6146 if len(field_path) < 2 or field_path[0] not in components:
! 6147 raise NotImplementedError(
6148 f"No derivative defined for '{type(self).__name__}' field: {field_path}."
6149 )
6150
6151 vjps: AutogradFieldMap = {}Lines 6153-6161 6153 comp_info = self._component_derivative_info(
6154 derivative_info=derivative_info, component=comp_name
6155 )
6156 if comp_info is None:
! 6157 continue
6158 comp_vjps = component._compute_derivatives(comp_info)
6159 for sub_path, value in comp_vjps.items():
6160 vjps[(comp_name, *sub_path)] = value |
groberts-flex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the addition! overall, looks like a good approach @marcorudolphflex
left a couple comments/questions!
493bef5 to
9cecb5d
Compare
groberts-flex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall, looks good! do you have plots for the outputs of the numerical tests you could share as well?
9cecb5d to
b001067
Compare
|
thanks for putting those plots together, they look interesting! Talking with Yannick a little bit this morning in our 1:1 and he was saying there is a test case he has for isotropic custom medium that is not working well for autograd. I think it's likely the underlying problem is in there, but might be good to get to the bottom of that. I think he is going to share that one with me and I'll have a look at it to see if I can spot anything. I'll also re-run my other numerical custom medium tests to see if maybe we had a regression somewhere. |
b001067 to
ddab4ab
Compare
Looks much better after the fix from https://github.com//pull/3113
|
4b3e783 to
a803b13
Compare
yaugenst-flex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcorudolphflex is there anything valuable in those bugbot reviews? otherwise feel free to merge
…stom-anisotropic-medium
a803b13 to
60fa31c
Compare











Greptile Overview
Greptile Summary
This PR adds autograd support for diagonal
AnisotropicMediumandCustomAnisotropicMedium, enabling gradient-based inverse design with anisotropic materials. The implementation delegates derivative computation to individual component media (xx, yy, zz) with proper field projection to ensure only the corresponding axis contributes to each gradient.Key changes:
_compute_derivativesmethod inAnisotropicMediumthat projects electric field maps to individual axes and delegates to component media_project_field_map_to_axisand_component_derivative_infofor field filteringThe implementation correctly inherits to
CustomAnisotropicMediumthrough the class hierarchy. Error handling raisesNotImplementedErrorfor unsupported paths (e.g., off-diagonal components), though explicit test coverage for this error case would strengthen the validation.Confidence Score: 4/5
Important Files Changed
File Analysis
_compute_derivativesmethod forAnisotropicMediumthat delegates to component media with proper field projection - minor test coverage gap for error casesAnisotropicMediumandCustomAnisotropicMediumSequence Diagram
sequenceDiagram participant User as User Code participant AAM as AnisotropicMedium participant Comp as Component Medium participant DI as DerivativeInfo User->>AAM: _compute_derivatives(derivative_info) AAM->>AAM: validate all paths exist in components loop for each component (xx, yy, zz) AAM->>AAM: _component_derivative_info(derivative_info, component) AAM->>AAM: _project_field_map_to_axis(E_der_map, axis) Note over AAM: Zero out non-matching field components<br/>(e.g., for xx: keep Ex, zero Ey, Ez) AAM->>DI: create filtered DerivativeInfo AAM->>Comp: component._compute_derivatives(comp_info) Comp-->>AAM: component gradients AAM->>AAM: prepend component name to gradient keys end AAM-->>User: combined gradients for all componentsContext used:
dashboard- Add tests for all public constructors and methods to confirm expected behavior, including effects on... (source)dashboard- Prefer pytest.mark.parametrize over manual for loops to define and run distinct test cases, reducing... (source)Note
Adds autograd support for diagonal anisotropy and validates it with focused tests.
AnisotropicMedium._compute_derivatives()to delegate gradients toxx/yy/zzcomponents, using_component_derivative_info()andDerivativeInfo.project_der_map_to_axis()to zero non-axis componentsDerivativeInfowithproject_der_map_to_axis()and use_zeros_likefor safe map projection.item()); new unit tests verify axis-specific gradients and D-map projection for conductivityCHANGELOG.mdto document the new autograd supportWritten by Cursor Bugbot for commit 60fa31c. This will update automatically on new commits. Configure here.