Skip to content

Autograd support for affine transformations #2490

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

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

yaugenst-flex
Copy link
Collaborator

@yaugenst-flex yaugenst-flex commented May 20, 2025

Supersedes #2475

@rahul-flex I rebased all the changes into this PR. Could you have a look and check if anything is missing?

Greptile Summary

Adds comprehensive autograd support for affine transformations in Tidy3D, enabling gradient-based optimization of geometric parameters like rotation, scaling, and translation.

  • Implemented new autograd support for box center/size parameters and rotation angles with chainable transformations in tidy3d/components/transformation.py
  • Refactored box derivative computation in tidy3d/components/geometry/base.py to use surface mesh integration instead of xarray-based approach
  • Added extensive test coverage in tests/test_components/test_box_chained_derivatives.py validating automatic differentiation against finite differences within 30% tolerance
  • Removed special case handling for Box geometries in tidy3d/web/api/autograd/autograd.py to unify permittivity computation across geometry types
  • Enhanced transform matrix construction to avoid in-place operations and support derivative computation via chain rule

@rahul-flex
Copy link
Contributor

rahul-flex commented May 20, 2025

Supersedes #2475

@rahul-flex I rebased all the changes into this PR. Could you have a look and check if anything is missing?

Thanks! Went through it. Looks good. Added an else condition. Ran the numerical tests. They pass!
All the pyetsts pass as well. However, I am seeing an increase in pytest execution time (usually takes ~20 mins locally.. currently took ~ 1hr). It seems this is taking long to run. I did not get a chance to get to the bottom of it. Felt I should mention it here.
Screenshot 2025-05-20 at 4 47 37 PM

@yaugenst-flex
Copy link
Collaborator Author

@greptileai

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.

8 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile

return (fun(x0 + δ) - fun(x0 - δ)) / (2 * δ)


def _assert_close(fd_val, ad_val, tag, axis=None, extra="", tol=0.35):
Copy link

Choose a reason for hiding this comment

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

style: The tolerance value 0.35 (35%) seems quite high for numerical comparison. Consider tightening or documenting justification.

return anp.sum(data.get_intensity("m").values)


def finite_diff_theta(center, size, eps, theta, axis, delta=1e-3):
Copy link

Choose a reason for hiding this comment

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

style: Default delta (1e-3) differs from global delta (0.016). Consider using the global delta by default.

@@ -66,7 +67,6 @@ def rotate_tensor(self, tensor: TensorReal) -> TensorReal:

if self.isidentity:
return tensor

return np.matmul(self.matrix, np.matmul(tensor, self.matrix.T))
Copy link

Choose a reason for hiding this comment

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

logic: rotate_tensor still uses np.matmul instead of anp.matmul - needs to be consistent with autograd usage.

Suggested change
return np.matmul(self.matrix, np.matmul(tensor, self.matrix.T))
return anp.matmul(self.matrix, anp.matmul(tensor, self.matrix.T))

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.

2 participants