Skip to content

Conversation

@weiliangjin2021
Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 commented Oct 13, 2025

This PR is to unblock some tests. Backend improvement will be worked on soon.

Greptile Overview

Updated On: 2025-10-13 19:04:51 UTC

Greptile Summary

This PR relaxes validation constraints for geometric operations by increasing the maximum allowed geometry count in a single structure from 100 to 5000. The change affects the MAX_GEOMETRY_COUNT constant in scene.py and is intended to support more complex ClipOperations that contain numerous geometric components. This modification unblocks failing tests that were hitting the previous geometric complexity limit. The change is properly documented in the CHANGELOG.md under the "Changed" category, indicating it's a modification to existing validation behavior rather than a bug fix. The PR serves as a temporary workaround while backend performance improvements are being developed to handle complex geometries more efficiently.

Changed Files
Filename Score Overview
CHANGELOG.md 5/5 Added changelog entry documenting the relaxation of geometry limits for ClipOperation
tidy3d/components/scene.py 4/5 Increased MAX_GEOMETRY_COUNT constant from 100 to 5000 to allow more complex geometric structures

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as it only relaxes validation constraints to enable previously blocked functionality
  • Score reflects the temporary nature of the change and potential performance implications of allowing 50x more geometries without corresponding backend improvements
  • Pay close attention to tidy3d/components/scene.py to ensure the increased limit doesn't introduce performance issues in production

Sequence Diagram

sequenceDiagram
    participant User
    participant Scene
    participant Validator
    participant Structure
    participant Medium
    participant GeometryGroup
    participant ClipOperation
    participant Logger

    User->>Scene: Create Scene with structures
    Scene->>Validator: Validate structures count
    
    alt Too many structures
        Validator->>Logger: Log warning about structures per medium
        alt Exceeds maximum
            Validator->>Scene: Raise SetupError
        end
    end

    Scene->>Validator: Validate number of geometries
    loop For each structure
        Validator->>Structure: Get geometry
        Structure->>GeometryGroup: Flatten groups if needed
        GeometryGroup->>ClipOperation: Count geometries
        alt Too many geometries
            Validator->>Scene: Raise SetupError
        end
    end

    Scene->>Scene: Create cached properties
    Scene->>Structure: Sort structures by priority
    Scene->>Scene: Build medium map
    
    User->>Scene: Call plot method
    Scene->>Scene: Get plot limits from bounds
    Scene->>Scene: Get structures for 2D box
    loop For each structure
        Scene->>Structure: Get geometry intersections
        Scene->>Medium: Get plotting parameters
        Scene->>Scene: Plot shape with medium
    end
    Scene->>Scene: Return matplotlib axes
Loading

Context used (3)

  • Rule from dashboard - Update the CHANGELOG.md file when making changes that affect user-facing functionality or fix bugs. (source)
  • Rule from dashboard - Use changelog categories correctly: "Fixed" for bug fixes, "Changed" for modifications to existing f... (source)
  • Rule from dashboard - Add new constants to the existing constants.py file rather than defining them locally. Be careful wi... (source)

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.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@weiliangjin2021 weiliangjin2021 added this pull request to the merge queue Oct 13, 2025
Merged via the queue into develop with commit ee99802 Oct 13, 2025
50 of 54 checks passed
@weiliangjin2021 weiliangjin2021 deleted the weiliang/FXC-3648-increase-allowed-structure-count-for-validator-rf branch October 13, 2025 20:54
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.

3 participants