Skip to content

Conversation

yaugenst-flex
Copy link
Collaborator

@yaugenst-flex yaugenst-flex commented Jun 18, 2025

Summary

This PR introduces the capability to compute gradients with respect to the conductivity of a CustomMedium.

Implementation Details

  • Generalizes the _derivative_field_cmp helper method with a new component parameter that can be set to 'real', 'imag', or 'complex'.
  • Implements the Vector-Jacobian Product (VJP) for conductivity (component='imag') by scaling the imaginary part of the complex permittivity derivative by 1 / (omega * epsilon_0).
  • Updates the _compute_derivatives dispatcher to handle the new "conductivity" parameter path and invoke the generalized helper method.
  • Adds test_custom_medium_conductivity_only_gradient to validate the gradient calculation for a CustomMedium with variable conductivity and constant permittivity.
  • Adds a test case structure, custom_med_with_conductivity, that contains both variable permittivity and conductivity.

Greptile Summary

Implements conductivity gradient calculations for CustomMedium by extending the permittivity gradient framework to handle real, imaginary, and complex components, enabling material conductivity optimization.

  • Added component parameter to _derivative_field_cmp to support 'real', 'imag', and 'complex' gradient types
  • Implemented conductivity Vector-Jacobian Product (VJP) by scaling imaginary permittivity derivative by 1/(omega * epsilon_0)
  • Added new test case custom_med_with_conductivity to validate combined permittivity and conductivity gradients
  • Refactored test code in test_autograd.py to improve maintainability with shared coordinate dictionaries

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.

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

Copy link
Contributor

github-actions bot commented Jun 18, 2025

Diff Coverage

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

  • tidy3d/components/medium.py (96.0%): Missing lines 3013

Summary

  • Total: 25 lines
  • Missing: 1 line
  • Coverage: 96%

tidy3d/components/medium.py

  3009             # for complex permittivity in eps_dataset, return the full complex derivative
  3010             E_der_dim_interp = E_der_dim_interp_complex
  3011         elif component == "imag":
  3012             # pure imaginary component (no conductivity conversion)
! 3013             E_der_dim_interp = E_der_dim_interp_complex.imag
  3014         else:
  3015             E_der_dim_interp = E_der_dim_interp_complex.real
  3016 
  3017         E_der_dim_interp = E_der_dim_interp.sum("f")

@momchil-flex momchil-flex added 2.9 will go into version 2.9.* rc2 2nd pre-release labels Jun 19, 2025
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.

LGTM

1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile

@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/conductivity-grads branch 3 times, most recently from 20bc219 to 7e50b67 Compare June 23, 2025 11:59
Copy link
Collaborator

@tylerflex tylerflex 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!

a really minor comment is that I notice that we sum("f") the E_der_dim_interp in each of the handlings of component. could be slightly more robust if w just sum that after constructing E_der_dim_interp.

@yaugenst-flex
Copy link
Collaborator Author

a really minor comment is that I notice that we sum("f") the E_der_dim_interp in each of the handlings of component. could be slightly more robust if w just sum that after constructing E_der_dim_interp.

good point, fixed

Copy link
Contributor

@groberts-flex groberts-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! not sure this would be in the scope of the PR or not but would love to have a numerical test for conductivity as well. I'm happy to take this on at some point, think I could modify the field and/or mode numerical tests we have to check conductivity (right now, they use custom medium for permittivity, but should be quick change to also test conductivity.

@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/conductivity-grads branch from 673ba6a to 13198e0 Compare June 27, 2025 07:30
@momchil-flex momchil-flex removed the rc2 2nd pre-release label Jul 14, 2025
@daquinteroflex
Copy link
Collaborator

daquinteroflex commented Jul 23, 2025

Hello! Is the plan to merge this by 2.9.0 ? We can add the .1 tag possbly if not?

@yaugenst-flex yaugenst-flex removed .1 2.9 will go into version 2.9.* labels Aug 13, 2025
@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/conductivity-grads branch 2 times, most recently from 8bedd7f to abaecd9 Compare August 18, 2025 11:45
Copy link
Contributor

@groberts-flex groberts-flex left a comment

Choose a reason for hiding this comment

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

overall looks good, couple of small comments.

did you want me to send you the fixed up numerical test to include here as well? or do you want to save that for a separate PR?

@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/conductivity-grads branch from abaecd9 to d1c44f3 Compare August 18, 2025 15:35
Introduces support for computing gradients with respect to the `conductivity` field in `CustomMedium`.

To achieve this, the derivative computation logic was generalized:
- The `_derivative_field_cmp` method is refactored to accept a `component` parameter ('real', 'imag', 'complex') to compute the VJP for different parts of the complex permittivity.
- For the 'imag' component, the derivative is scaled by `1 / (omega * epsilon_0)` to convert from derivative w.r.t. complex permittivity to derivative w.r.t. conductivity.
- The `_compute_derivatives` dispatcher now handles the new `"conductivity"` parameter path.
- A new autograd test is added to validate the conductivity gradient calculation on a `CustomMedium` with constant permittivity.
@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/conductivity-grads branch from 0663fae to c73c63f Compare August 18, 2025 19:45
@yaugenst-flex yaugenst-flex added this pull request to the merge queue Aug 18, 2025
Merged via the queue into develop with commit fba5441 Aug 18, 2025
24 checks passed
@yaugenst-flex yaugenst-flex deleted the yaugenst-flex/conductivity-grads branch August 18, 2025 21:30
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.

5 participants