Skip to content

Conversation

yaugenst-flex
Copy link
Collaborator

@yaugenst-flex yaugenst-flex commented Sep 10, 2025

Greptile Summary

Updated On: 2025-09-10 08:44:14 UTC

This PR implements backwards-compatible component modeler run() functionality with autograd support. The changes consolidate the component modeler execution path under the unified tidy3d.web.run() API while maintaining backwards compatibility for existing code.

The core changes involve three main areas:

  1. API Consolidation: The autograd run() function in tidy3d/web/api/autograd/autograd.py is modified to accept ComponentModeler objects by broadening the simulation parameter type from td.Simulation to typing.Any. This enables the function to handle both regular simulations and component modelers through a unified interface.

  2. Component Modeler Routing: New routing logic detects ComponentModeler instances and routes them to the appropriate smatrix local run function when they contain autograd-valid simulations or when local_gradient is enabled. This leverages the existing smatrix infrastructure while enabling autograd capabilities.

  3. Documentation Cleanup: The migration documentation is updated to remove references to deprecated legacy functionality, specifically removing the "legacy run command" section that documented tidy3d.plugins.smatrix.run.run. This aligns the documentation with the deprecation of the legacy helper function.

The integration fits well with the existing codebase architecture by reusing established patterns for type checking and routing to different execution paths. The changes maintain the existing td.Simulation autograd path unchanged, ensuring no breaking changes for current users while extending functionality to component modelers. The test updates demonstrate that the new API works correctly, with modeler.run() now returning smatrix data directly instead of requiring an additional smatrix() call.

Important Files Changed

Changed Files
Filename Score Overview
docs/api/plugins/smatrix_migration.rst 5/5 Removes deprecated legacy run command documentation to align with API modernization
tests/test_plugins/smatrix/test_component_modeler_autograd.py 4/5 Updates tests to use new web.run() API and validates autograd functionality with component modelers
tidy3d/web/api/autograd/autograd.py 4/5 Adds component modeler support to autograd run function with type broadening and routing logic

Confidence score: 4/5

  • This PR is generally safe to merge with some attention needed for the type system changes
  • Score reflects solid architectural design but introduces runtime type checking instead of compile-time validation
  • Pay close attention to the autograd.py file for potential type safety implications

Sequence Diagram

sequenceDiagram
    participant User
Loading

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.

Reviewing changes made in this pull request

@daquinteroflex daquinteroflex added rc1 1st pre-release 2.10 labels Sep 10, 2025
@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/cm-autograd-web branch 2 times, most recently from 538ba3f to ca37a45 Compare September 10, 2025 09:56
Copy link
Contributor

github-actions bot commented Sep 10, 2025

Diff Coverage

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

  • tidy3d/plugins/smatrix/component_modelers/base.py (100%)
  • tidy3d/plugins/smatrix/run.py (100%)
  • tidy3d/web/api/autograd/autograd.py (100%)

Summary

  • Total: 19 lines
  • Missing: 0 lines
  • Coverage: 100%

Copy link
Contributor

@groberts-flex groberts-flex left a comment

Choose a reason for hiding this comment

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

looks good to me, just a couple small questions! Thanks @yaugenst-flex

@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/cm-autograd-web branch 2 times, most recently from 8b4ec67 to 925d99e Compare September 10, 2025 14:57
@tylerflex tylerflex self-requested a review September 10, 2025 15:05
@yaugenst-flex yaugenst-flex added this pull request to the merge queue Sep 10, 2025
auto-merge was automatically disabled September 10, 2025 15:11

Pull Request is not mergeable

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 10, 2025
@yaugenst-flex yaugenst-flex force-pushed the yaugenst-flex/cm-autograd-web branch from 925d99e to 3ef5385 Compare September 10, 2025 16:11
@yaugenst-flex yaugenst-flex added this pull request to the merge queue Sep 10, 2025
Merged via the queue into develop with commit 6a04e86 Sep 10, 2025
24 checks passed
@yaugenst-flex yaugenst-flex deleted the yaugenst-flex/cm-autograd-web branch September 10, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.10 rc1 1st pre-release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants