Skip to content

Conversation

@mahlau-flex
Copy link
Contributor

@mahlau-flex mahlau-flex commented Nov 28, 2025

Added symmetry functions according to https://flow360.atlassian.net/browse/FXC-4347

Greptile Overview

Greptile Summary

This PR adds three symmetry functions (symmetrize_mirror, symmetrize_rotation, symmetrize_diagonal) for topology optimization in inverse design workflows. These functions enforce known symmetries during optimization by averaging arrays with their transformed versions.

  • symmetrize_mirror: averages with axis-flipped versions for mirror symmetry
  • symmetrize_rotation: averages over 90° rotations for 4-fold rotational symmetry
  • symmetrize_diagonal: averages with transpose for diagonal symmetry

The implementation uses autograd-compatible operations (slicing, transpose) to maintain gradient flow. Tests verify gradient correctness, numerical accuracy, and error handling.

Key Issues:

  • Uses generic Exception instead of ValueError for input validation (violates codebase pattern)
  • Copy-paste error in symmetrize_mirror docstring mentions "ramp projection"

Confidence Score: 4/5

  • Safe to merge after fixing exception types and docstring
  • Implementation is mathematically sound with comprehensive tests, but uses wrong exception type for validation (should be ValueError not Exception) and has a docstring copy-paste error. These are straightforward fixes.
  • tidy3d/plugins/autograd/invdes/symmetries.py needs error handling corrections

Important Files Changed

File Analysis

Filename Score Overview
tidy3d/plugins/autograd/invdes/symmetries.py 3/5 New symmetry functions with minor issues: uses generic Exception instead of ValueError for validation, and has copy-paste error in docstring
tests/test_plugins/autograd/invdes/test_symmetries.py 5/5 Comprehensive test coverage with gradient checks, value verification, invariance tests, and error handling

Sequence Diagram

sequenceDiagram
    participant User as User/Optimizer
    participant Sym as symmetrize_* functions
    participant AG as Autograd
    participant Array as NDArray
    
    Note over User,Array: Topology Optimization Flow
    
    User->>Sym: Call symmetry function (array, params)
    activate Sym
    
    Sym->>Array: Validate array shape (ndim=2)
    alt Invalid shape
        Array-->>Sym: Raise Exception
        Sym-->>User: Error
    end
    
    alt symmetrize_mirror
        Sym->>Array: Flip array along axis
        Array-->>Sym: Flipped array
        Sym->>Sym: Average original + flipped
    else symmetrize_rotation
        Sym->>Array: Apply rotations (90°, 180°, 270°)
        Array-->>Sym: Rotated arrays
        Sym->>Sym: Average 4 rotations
    else symmetrize_diagonal
        Sym->>Array: Transpose array
        alt Anti-diagonal
            Array-->>Sym: Flipped transpose
        else Main diagonal
            Array-->>Sym: Regular transpose
        end
        Sym->>Sym: Average original + transpose
    end
    
    Sym->>AG: Return symmetrized array
    deactivate Sym
    
    Note over User,AG: Gradient Flow (Backpropagation)
    
    User->>AG: Compute gradients
    AG->>Sym: Backward pass through symmetry ops
    Sym->>Array: Compute VJP (slicing, transpose)
    Array-->>AG: Gradient tensors
    AG-->>User: Gradients w.r.t. input
Loading

Context used:

  • Rule from dashboard - Use log.error instead of assert statements for error handling in production code. (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.

5 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@mahlau-flex mahlau-flex force-pushed the FXC-4347-Add-symmetry-helper-functions-for-topology-optimization-parameterizations branch 2 times, most recently from 66ece12 to ffeb45d Compare November 28, 2025 09:01
@github-actions
Copy link
Contributor

github-actions bot commented Nov 28, 2025

Diff Coverage

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

  • tidy3d/plugins/autograd/invdes/init.py (100%)
  • tidy3d/plugins/autograd/invdes/symmetries.py (92.3%): Missing lines 60,106,168

Summary

  • Total: 40 lines
  • Missing: 3 lines
  • Coverage: 92%

tidy3d/plugins/autograd/invdes/symmetries.py

  56         if ax == 0:
  57             return arr[::-1, :]
  58         elif ax == 1:
  59             return arr[:, ::-1]
! 60         return arr
  61 
  62     # Case 1: Symmetrize along both axes
  63     if isinstance(axis, Sequence):
  64         # Symmetrize along axis 0

  102         >>>     [4, 1, 4]
  103         >>> ])))
  104     """
  105     if array.ndim != 2:
! 106         raise ValueError(f"Invalid array shape: {array.shape}. Need 2d array.")
  107 
  108     if array.shape[0] != array.shape[1]:
  109         raise ValueError(
  110             f"Invalid array shape: {array.shape}. Array must be square for rotational symmetry."

  164         >>>     [4, 4]
  165         >>> ])))
  166     """
  167     if array.ndim != 2:
! 168         raise ValueError(f"Invalid array shape: {array.shape}. Need 2d array.")
  169 
  170     if array.shape[0] != array.shape[1]:
  171         raise ValueError(
  172             f"Invalid array shape: {array.shape}. Array must be square for diagonal symmetry."

@mahlau-flex mahlau-flex force-pushed the FXC-4347-Add-symmetry-helper-functions-for-topology-optimization-parameterizations branch from ffeb45d to 89c3f6a Compare November 28, 2025 09:49
Copy link
Collaborator

@yaugenst-flex yaugenst-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 @mahlau-flex this looks great! Two general comments:

  1. Could you also add these to docs/api/plugins/autograd.rst so they are listed in the docs
  2. It would be nice to have an example in the docstrings, which for these simple helpers would be pretty useful and easy enough to do?

@mahlau-flex mahlau-flex force-pushed the FXC-4347-Add-symmetry-helper-functions-for-topology-optimization-parameterizations branch from 89c3f6a to bcab6fd Compare November 28, 2025 12:58
@mahlau-flex
Copy link
Contributor Author

@yaugenst-flex I added the requested features. Let me know if you like the style of the examples.

@yaugenst-flex
Copy link
Collaborator

@yaugenst-flex I added the requested features. Let me know if you like the style of the examples.

Looks great! Thanks!

@yaugenst-flex yaugenst-flex added this pull request to the merge queue Nov 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2025
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