add source differentiation #2710
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
tidy3d/plugins/autograd/README.md
Outdated
|
|
||
| def objective(source_amplitude): | ||
| # Create traced field data for the source | ||
| field_data = source_amplitude * np.ones((10, 10, 1, 1)) |
There was a problem hiding this comment.
syntax: Missing import for np - the example uses np.ones() but only imports anp
| field_data = source_amplitude * np.ones((10, 10, 1, 1)) | |
| field_data = source_amplitude * anp.ones((10, 10, 1, 1)) |
tidy3d/plugins/autograd/README.md
Outdated
| def objective(source_amplitude): | ||
| # Create traced field data for the source | ||
| field_data = source_amplitude * np.ones((10, 10, 1, 1)) | ||
| scalar_field = td.ScalarFieldDataArray(field_data, coords=coords) |
There was a problem hiding this comment.
syntax: Variable coords is undefined in the example code
tidy3d/components/base.py
Outdated
| if isinstance(starting_paths[0], str): | ||
| starting_paths = (starting_paths,) |
There was a problem hiding this comment.
logic: The type check isinstance(starting_paths[0], str) could fail with IndexError if starting_paths is an empty tuple. Should check starting_paths and isinstance(starting_paths[0], str) instead.
| if isinstance(starting_paths[0], str): | |
| starting_paths = (starting_paths,) | |
| if starting_paths and isinstance(starting_paths[0], str): | |
| starting_paths = (starting_paths,) |
tidy3d/plugins/autograd/README.md
Outdated
| sim = td.Simulation( | ||
| size=(2.0, 2.0, 2.0), | ||
| sources=[custom_source], | ||
| monitors=[td.FieldMonitor(size=(1.0, 1.0, 0.0), center=(0, 0, 0), freqs=[2e14])] |
There was a problem hiding this comment.
logic: Monitor name should match the name used in line 266 (load_field_monitor expects a monitor name)
| monitors=[td.FieldMonitor(size=(1.0, 1.0, 0.0), center=(0, 0, 0), freqs=[2e14])] | |
| monitors=[td.FieldMonitor(size=(1.0, 1.0, 0.0), center=(0, 0, 0), freqs=[2e14], name="field_monitor")] |
tidy3d/web/api/autograd/autograd.py
Outdated
| derivative_info = DerivativeInfo( | ||
| paths=source_paths, | ||
| E_der_map={}, # Placeholder - source-specific field maps needed | ||
| D_der_map={}, # Placeholder - source-specific field maps needed | ||
| E_fwd=None, # Placeholder - source-specific forward fields needed | ||
| E_adj=None, # Placeholder - source-specific adjoint fields needed | ||
| D_fwd=None, # Placeholder - source-specific forward fields needed | ||
| D_adj=None, # Placeholder - source-specific adjoint fields needed | ||
| eps_data=None, # Not applicable for sources | ||
| eps_in=None, # Not applicable for sources | ||
| eps_out=None, # Not applicable for sources | ||
| eps_background=None, # Not applicable for sources | ||
| frequencies=np.array([]), # Placeholder | ||
| eps_no_structure=None, # Not applicable for sources | ||
| eps_inf_structure=None, # Not applicable for sources | ||
| bounds=((0, 0, 0), (0, 0, 0)), # Placeholder | ||
| bounds_intersect=((0, 0, 0), (0, 0, 0)), # Placeholder | ||
| ) |
There was a problem hiding this comment.
logic: DerivativeInfo is created with placeholder/empty values for all source-specific fields. This will likely cause issues when _compute_derivatives is actually called on sources.
tidy3d/web/api/autograd/autograd.py
Outdated
| return sim_fields_vjp | ||
| else: | ||
| # Fallback for sources without _compute_derivatives method | ||
| td.log.warning(f"Source {source_index} does not have _compute_derivatives method") |
There was a problem hiding this comment.
style: Warning message uses f-string with Source {source_index} but doesn't identify which source type or provide enough context for debugging.
Context Used: Rule - Make log messages and warnings informative by including relevant context and enclosing variable names in single quotes. (link)
tidy3d/components/source/current.py
Outdated
|
|
||
| # For now, return placeholder gradients with expected structure | ||
| # This is a placeholder for future implementation | ||
| import tidy3d as td |
There was a problem hiding this comment.
style: Consider using td.log.debug() instead of td.log.info() since this is a placeholder message that will appear frequently during development
adds ability to diff w.r.t. sources. but doesn't actually implement the VJP yet!
Greptile Summary
This PR adds foundational infrastructure for source differentiation in Tidy3D's autograd system, enabling automatic differentiation with respect to source parameters like
CustomCurrentSource. The changes establish the framework to treat sources as differentiable components alongside existing structure differentiation capabilities.The implementation follows the established autograd pattern in the codebase where differentiable components implement a
_compute_derivativesmethod and are tracked through path-based systems. The main architectural changes include:starting_pathto multiplestarting_pathsthroughout the autograd pipeline to handle both structures and sources simultaneouslyCustomCurrentSource._compute_derivatives()method and updated the web API autograd system to recognize sources as traceable componentsThe changes integrate cleanly with the existing autograd architecture by extending the path-based field tracking system used for structures. The
_strip_traced_fieldsmethod signature was updated across the codebase to support multiple starting paths, enabling efficient batch processing of both structure and source parameters in a single operation.PR Description Notes:
Important Files Changed
Changed Files
_compute_derivativesmethod toCustomCurrentSourcewith proper logging and zero gradient returns_strip_traced_fieldsto accept multiple starting paths instead of single path for batch processingClipOperationvalidator to use newstarting_pathsparameter in_strip_traced_fieldscall_strip_traced_fieldsmethod signatureConfidence score: 4/5