flexcompute / tidy3d

Fast electromagnetic solver (FDTD) at scale.
https://docs.flexcompute.com/projects/tidy3d/en/latest/
GNU Lesser General Public License v2.1
179 stars 40 forks source link

implementing interpolation option for `plot_field` method #1964

Open FilipeFcp opened 1 week ago

FilipeFcp commented 1 week ago

Hi @tylerflex,

I’ve implemented the interpolation option. Here's how I approached it:

The default is set to False, so it generates a regular plot. If set to True, it interpolates with 200 x 200 points. If a tuple (a, b) is provided, it interpolates with a x b points. If an int a is provided, it interpolates with a x a points. I added an assertion to prevent users from interpolating more than 1000 points in any dimension to avoid potential notebook crashes due to memory limitations.

Let me know if this approach is too complex; I can simplify it if needed, like just True or False.

Fixes #1952

tylerflex commented 1 week ago

Hi @FilipeFcp Thanks for this! The interpolation approach makes some sense, but also introduces these new parameters (like number of points). I wonder if instead we should include a way to downsample the fields?

In xarray there is a convenient way to grab every n data point by using slice https://docs.xarray.dev/en/latest/user-guide/indexing.html

>>> import xarray as xr
>>> da = xr.DataArray([1, 2, 3], [("x", [0, 1, 2])])
>>> da.isel(x=slice(None, None, 2))
<xarray.DataArray (x: 2)> Size: 16B
array([1, 3])
Coordinates:
  * x        (x) int64 16B 0 2

perhaps we could instead allow the user to input the n for slice(None, None, n) (with default of 1)? and use that to downsample in all directions?

Just a thought let me know how you think that would work or if you prefer the interpolation approach

FilipeFcp commented 6 days ago

What I had in mind was a way to add more data points, making it easier to create smoother visualizations. For example, moving from this:

image

To this:

image

I understand that the slice option is useful for quickly plotting a large dataset, but could it be used to generate a smoother image? However, if users are most likely looking to downsample datasets before plotting rather than generating more points for visualization purposes, the slice option would make more sense.

tylerflex commented 6 days ago

ah ok, I misunderstood. Maybe in the same sprit, it could be nice if the interpolation argument was a float = 1 that just specified the ratio of points in the interpolation coords to the number of points in the raw data? for example nterpolation=2 would mean we sample the points at 2x resolution of the supplied data. Would this make sense?

The only thing about the current approach I think is not ideal is that it requires the hardcoding of these values (200x200) whereas the data might have a different aspect ratio, so it could lead to weird results.

Are there other examples of interpolation in plotting and what do they do?

Also, could you include a changelog item under Added for the final PR? thanks

FilipeFcp commented 2 days ago

Hi @tylerflex, your float approach is actually way better.

Ideally, the user could be able to pass the shading argument for plotting, but it seems to be bugged at the moment: https://github.com/pydata/xarray/issues/3002.

tylerflex commented 2 days ago

looks like they could also try this?

https://github.com/pydata/xarray/issues/3002#issuecomment-735419937

FilipeFcp commented 1 day ago

Oh... I saw this comment, but I tried with infer_interval = True not False. Yes, it works with False.

I think we can pass the shading argument through - plot_field -> plot_field_monitor_data -> plot_scalar_array -> field_data.plot. Or create a **args in plot_scalar_array and input it into field_data.plot.

tylerflex commented 1 day ago

Oh... I saw this comment, but I tried with infer_interval = True not False. Yes, it works with False.

Great

I think we can pass the shading argument through - plot_field -> plot_field_monitor_data -> plot_scalar_array -> field_data.plot. Or create a **args in plot_scalar_array and input it into field_data.plot.

yea I think either a shading argument or just exploring all of the **kwargs for xarray would work. I dont have a strong preference. what do you think is better? will this be used often you think?