Skip to content

Electric field monitor for Charge #2566

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

marc-flex
Copy link
Contributor

@marc-flex marc-flex commented Jun 12, 2025

Greptile Summary

Adds electric field monitoring capabilities to the Charge simulation module, enabling visualization and analysis of steady-state electric fields through a new SteadyElectricFieldMonitor class and associated data structures.

  • Added SteadyElectricFieldMonitor in tidy3d/components/tcad/monitors/charge.py to monitor Ex, Ey, Ez components
  • Implemented SteadyElectricFieldData class in tidy3d/components/tcad/data/monitor_data/charge.py handling electric field data with proper unit conversion (V/μm)
  • Added _cell_to_point_data utility in UnstructuredGridDataset for VTK-based cell-to-point data conversion
  • Added comprehensive test suite validating electric field monitoring for tetrahedral and triangular grids

@marc-flex marc-flex self-assigned this Jun 12, 2025
@marc-flex marc-flex marked this pull request as ready for review June 13, 2025 14:15
Copy link
Contributor

github-actions bot commented Jun 13, 2025

Diff Coverage

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

  • tidy3d/components/data/data_array.py (100%)
  • tidy3d/components/data/unstructured/base.py (42.9%): Missing lines 629-631,633
  • tidy3d/components/tcad/data/monitor_data/charge.py (100%)
  • tidy3d/components/tcad/monitors/charge.py (100%)

Summary

  • Total: 43 lines
  • Missing: 4 lines
  • Coverage: 90%

tidy3d/components/data/unstructured/base.py

  625         vtk_obj,
  626     ):
  627         """Get point data values from a VTK object."""
  628 
! 629         cellDataToPointData = vtk["mod"].vtkCellDataToPointData()
! 630         cellDataToPointData.SetInputData(vtk_obj)
! 631         cellDataToPointData.Update()
  632 
! 633         return cellDataToPointData.GetOutput()
  634 
  635     @classmethod
  636     @requires_vtk
  637     def _get_values_from_vtk(

@marc-flex marc-flex force-pushed the marc/electric_field_monitor branch 2 times, most recently from a22f358 to 8ff1ec8 Compare June 13, 2025 15:54
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.

7 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile

@marc-flex marc-flex force-pushed the marc/electric_field_monitor branch from 61b4a48 to 50d69e3 Compare June 16, 2025 10:32
@momchil-flex momchil-flex added 2.9 will go into version 2.9.* rc2 2nd pre-release labels Jun 19, 2025
@dbochkov-flexcompute
Copy link
Contributor

Looks good overall, but I wanted to discuss whether we should store Ex, Ey, and Ez separately or in a single Triangular/Tetrahedral-GridDataset. In FDTD field monitors we do store them separately, but I guess one big reason to do that there is that Ex, Ey, and Ez can live on different grids (in fact, it was default up until some point). Moreover, storing cartesian grid info typically doesn't require much memory in comparison to actual field data. For unstructured datasets storing grid info does actually take more memory that a single field, and it seems like at least in this PR Ex, Ey, and Ez are stored at the same locations. Do you see any other arguments towards storing them separately or as a single xarray?

In my surface monitor PR I decided to go the single-array route and use indexed data array with additional coord "axis" https://github.com/flexcompute/tidy3d/pull/2360/files#diff-834da9ec0c6f019fc8dde7c9b3cd1d2eafb780dadfadb21e94c0a313be526076R1256-R1285. One other smaller advantage is that it makes calculating the magnitude of vector field and producing a new unstructured dataset for that very straightforward

def norm(self, dim) -> UnstructuredDataset:
"""Compute vector norm along a given dimension."""
return self.updated_copy(values=np.sqrt(self.values.dot(self.values.conj(), dim=dim).real))

@marc-flex
Copy link
Contributor Author

Yes, that makes sense! Will do that. Thanks @dbochkov-flexcompute

@marc-flex
Copy link
Contributor Author

marc-flex commented Jun 24, 2025

@dbochkov-flexcompute I just realized that we're already storing multiple solution in each of the Ex, Ey, Ez fields since it can have multiple voltages. Though currently it only stores one voltage in Conduction, it can store multiple in Charge. I'm wondering what approach we should follow here. Add another index to the data so that we have voltage and component?

@dbochkov-flexcompute
Copy link
Contributor

@dbochkov-flexcompute I just realized that we're already storing multiple solution in each of the Ex, Ey, Ez fields since it can have multiple voltages. Though currently it only stores one voltage in Conduction, it can store multiple in Charge. I'm wondering what approach we should follow here. Add another index to the data so that we have voltage and component?

yeah, I think that would make sense

@marc-flex
Copy link
Contributor Author

@dbochkov-flexcompute I updated this to use the same data array class you mentioned. There will probably be conflict when either this or your branch gets merged though but it should be pretty straightforward.

@marc-flex marc-flex force-pushed the marc/electric_field_monitor branch from ed5576e to 9a91727 Compare June 25, 2025 19:58
Copy link
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Thanks. Minor comments, also wondering what is your plan for the related current density monitor?

Monitor tests
Remove IndexedFieldDataArray
@marc-flex marc-flex force-pushed the marc/electric_field_monitor branch from ae9d57c to ca475c6 Compare June 27, 2025 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 will go into version 2.9.* Charge rc2 2nd pre-release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants