Skip to content

Conversation

@marc-flex
Copy link
Contributor

@marc-flex marc-flex commented Dec 22, 2025

Fix scene::plot_structures_porperty with property=doping when the doping is defined with SpatialDataArrays that don't have overlapping coordinates

Greptile Summary

This PR fixes a bug in the plot_structures_property method where plotting doping with SpatialDataArrays that have non-overlapping coordinates would fail. The fix adds bounds_error=False and fill_value=0 to the interpolation kwargs, allowing out-of-bounds points to be handled gracefully by setting them to 0 instead of raising an error. This change follows the existing interpolation pattern used elsewhere in the codebase (monitor_data.py).

Confidence Score: 5/5

  • This PR is safe to merge with no concerns - it's a minimal, targeted fix that addresses the previous review comment correctly.
  • The fix is minimal and focused: adding bounds_error=False and fill_value=0 to interpolation kwargs. This directly addresses the issue where scipy's RegularGridInterpolator would fail on out-of-bounds points. The implementation matches established patterns elsewhere in the codebase (monitor_data.py). The change is well-tested by existing test cases (test_plot_property) which specifically test plotting with SpatialDataArray doping. No new logic branches or complex code paths are introduced.
  • No files require special attention

Important Files Changed

Filename Overview
tidy3d/components/scene.py Fixes interpolation issue in plot_structures_property by adding bounds_error=False and fill_value=0 to kwargs when interpolating doping data with non-overlapping coordinates. This prevents scipy's RegularGridInterpolator from raising errors for out-of-bounds points.

Sequence Diagram

sequenceDiagram
    participant User
    participant plot_structures_property
    participant interp_method
    participant RegularGridInterpolator
    
    User->>plot_structures_property: Call with SpatialDataArray doping
    plot_structures_property->>plot_structures_property: Create 100x100 grid for plotting
    plot_structures_property->>interp_method: interp(**struct_coords, method="nearest", kwargs={...})
    Note over interp_method: Before fix: bounds_error=True (default)<br/>Grid points may fall outside data bounds
    interp_method->>RegularGridInterpolator: Create interpolator (bounds_error=True)
    RegularGridInterpolator-->>interp_method: ValueError if out-of-bounds!
    
    Note over plot_structures_property: After fix: bounds_error=False
    interp_method->>RegularGridInterpolator: Create interpolator (bounds_error=False, fill_value=0)
    RegularGridInterpolator->>RegularGridInterpolator: Out-of-bounds points → fill_value=0
    RegularGridInterpolator-->>interp_method: Interpolated data with zeros for out-of-bounds
    interp_method-->>plot_structures_property: Successfully interpolated data
    plot_structures_property-->>User: Plot rendered successfully
Loading

Note

Prevents interpolation errors when plotting doping from SpatialDataArray with non-overlapping coordinates by tolerating out-of-bounds samples and filling them with zeros.

  • In scene.py (_pcolormesh_shape_doping_box), updates data_2D.interp(..., method="nearest", kwargs={"bounds_error": False, "fill_value": 0}) to avoid failures and render out-of-bounds regions as 0
  • Change is localized to doping visualization; no other logic or files modified

Written by Cursor Bugbot for commit 6e07d79. This will update automatically on new commits. Configure here.

@marc-flex marc-flex self-assigned this Dec 22, 2025
Copilot AI review requested due to automatic review settings December 22, 2025 09:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug in scene::plot_structures_property when plotting doping properties where doping is defined with SpatialDataArrays that have non-overlapping coordinates. The fix adds a fill_value=0 parameter to the interpolation call to handle coordinates outside the data range.

Key Changes

  • Added kwargs={"fill_value": 0} to the interp() method call for SpatialDataArray doping data interpolation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@marc-flex
Copy link
Contributor Author

@greptile update summary

@greptile-apps
Copy link

greptile-apps bot commented Dec 22, 2025

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

@marc-flex marc-flex force-pushed the bug/FXC-4341/fix_doping_plot_spatial_data_array branch from 19c3acc to e1327f6 Compare December 22, 2025 10:15
@marc-flex marc-flex requested a review from momchil-flex January 9, 2026 07:41
@yaugenst-flex yaugenst-flex force-pushed the bug/FXC-4341/fix_doping_plot_spatial_data_array branch from e1327f6 to 40509e5 Compare January 9, 2026 08:46
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

Diff Coverage

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

  • tidy3d/components/scene.py (0.0%): Missing lines 2161

Summary

  • Total: 1 line
  • Missing: 1 line
  • Coverage: 0%

tidy3d/components/scene.py

Lines 2157-2165

  2157                 data_is_2d = any(dim_size <= 1 for _, dim_size in doping.sizes.items())
  2158                 if not data_is_2d:
  2159                     selector = {"xyz"[normal_axis_ind]: normal_position}
  2160                     data_2D = doping.sel(**selector)
! 2161                 contrib = data_2D.interp(
  2162                     **struct_coords,
  2163                     method="nearest",
  2164                     kwargs={"bounds_error": False, "fill_value": 0},
  2165                 )

@yaugenst-flex yaugenst-flex added this pull request to the merge queue Jan 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 9, 2026
@marc-flex marc-flex added this pull request to the merge queue Jan 9, 2026
@marc-flex marc-flex removed this pull request from the merge queue due to a manual request Jan 9, 2026
@marc-flex marc-flex force-pushed the bug/FXC-4341/fix_doping_plot_spatial_data_array branch from 40509e5 to bc6648e Compare January 9, 2026 11:40
@marc-flex marc-flex enabled auto-merge January 9, 2026 11:41
@marc-flex marc-flex added this pull request to the merge queue Jan 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 9, 2026
@marc-flex marc-flex force-pushed the bug/FXC-4341/fix_doping_plot_spatial_data_array branch 2 times, most recently from f527fef to 1e664e0 Compare January 9, 2026 14:19
@marc-flex marc-flex enabled auto-merge January 9, 2026 14:19
@marc-flex marc-flex added this pull request to the merge queue Jan 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 9, 2026
- Fixes scene::plot_structures_porperty with property=doping when the doping is
  defined with SpatialDataArrays that don't have overlapping coordinates
@marc-flex marc-flex force-pushed the bug/FXC-4341/fix_doping_plot_spatial_data_array branch from 1e664e0 to 6e07d79 Compare January 9, 2026 15:54
@marc-flex marc-flex enabled auto-merge January 9, 2026 15:54
@marc-flex marc-flex added this pull request to the merge queue Jan 9, 2026
Merged via the queue into develop with commit 976b00c Jan 9, 2026
33 of 37 checks passed
@marc-flex marc-flex deleted the bug/FXC-4341/fix_doping_plot_spatial_data_array branch January 9, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants