Skip to content

Conversation

@yaugenst-flex
Copy link
Collaborator

@yaugenst-flex yaugenst-flex commented Oct 7, 2025

Greptile Overview

Updated On: 2025-10-07 10:24:42 UTC

Summary

This PR removes duplicate code in the autograd module by consolidating redundant files and functions. The changes eliminate three duplicate files (`ops_forward.py`, `ops_backward.py`, and `constants_local.py`) and remove deprecated function shims from `autograd.py`.

The consolidation moves functionality from:

  • ops_forward.pyforward.py (functions like setup_fwd() and postprocess_fwd())
  • ops_backward.pybackward.py (functions like setup_adj() and postprocess_adj())
  • constants_local.pyconstants.py (autograd-related constants)

The main autograd.py file is updated to import from the consolidated modules (.forward, .backward, .constants) instead of the removed duplicates. Additionally, deprecated backward-compatibility shims (_compute_eps_array and _slice_field_data) are removed from autograd.py since the actual implementations exist in the backward.py module.

This refactoring follows the DRY (Don't Repeat Yourself) principle by maintaining a single source of truth for each function and constant. The autograd functionality for automatic differentiation in electromagnetic simulations remains fully intact, but the module structure is now cleaner and more maintainable. All critical adjoint simulation processing capabilities, forward pass setup, and constant definitions are preserved in their consolidated locations.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/web/api/autograd/autograd.py 5/5 Removed deprecated function shims and unused numpy import
tidy3d/web/api/autograd/ops_forward.py 5/5 Deleted entire file - functions consolidated into forward.py
tidy3d/web/api/autograd/ops_backward.py 5/5 Deleted entire file - functions consolidated into backward.py
tidy3d/web/api/autograd/constants_local.py 4/5 Deleted entire file - constants consolidated into constants.py

Confidence score: 5/5

  • This PR is safe to merge with minimal risk as it only removes duplicate code without changing functionality
  • Score reflects that this is a straightforward code cleanup with existing implementations preserved in consolidated modules
  • No files require special attention as all changes maintain existing functionality through proper consolidation

Sequence Diagram

sequenceDiagram
    participant User
    participant "web.api.autograd.run" as Run
    participant "is_valid_for_autograd" as Validator
    participant "_run" as RunImpl
    participant "setup_run" as Setup
    participant "_run_primitive" as Primitive
    participant "setup_fwd" as FwdSetup
    participant "_run_tidy3d" as Engine
    participant "postprocess_fwd" as FwdPost
    participant "postprocess_run" as PostProcess

    User->>Run: "run(simulation, task_name, ...)"
    Run->>Validator: "is_valid_for_autograd(simulation)"
    Validator-->>Run: "validation result"
    
    alt is autograd valid
        Run->>RunImpl: "_run(simulation, task_name, ...)"
        RunImpl->>Setup: "setup_run(simulation)"
        Setup-->>RunImpl: "traced_fields_sim"
        
        alt has traced fields
            RunImpl->>Primitive: "_run_primitive(traced_fields_sim, ...)"
            Primitive->>FwdSetup: "setup_fwd(sim_fields, sim_original, ...)"
            FwdSetup-->>Primitive: "sim_combined"
            
            alt local_gradient
                Primitive->>Engine: "_run_tidy3d(sim_combined, ...)"
                Engine-->>Primitive: "sim_data_combined, task_id"
                Primitive->>FwdPost: "postprocess_fwd(sim_data_combined, ...)"
                FwdPost-->>Primitive: "field_map"
            else server gradient
                Primitive->>Engine: "_run_tidy3d(sim_original, ...)"
                Engine-->>Primitive: "sim_data_orig, task_id_fwd"
                Note over Primitive: "Store task_id_fwd and sim_data_orig in aux_data"
                Primitive-->>Primitive: "extract field_map from sim_data_orig"
            end
            
            Primitive-->>RunImpl: "field_map"
            RunImpl->>PostProcess: "postprocess_run(traced_fields_data, aux_data)"
            PostProcess-->>RunImpl: "sim_data with tracers"
        else no traced fields
            Note over RunImpl: "Log warning about no tracers"
            RunImpl->>Engine: "_run_tidy3d(simulation, ...)"
            Engine-->>RunImpl: "sim_data, task_id"
        end
        
        RunImpl-->>Run: "simulation_data"
    else not autograd valid
        Run->>Engine: "run_webapi(simulation, ...)"
        Engine-->>Run: "simulation_data"
    end
    
    Run-->>User: "simulation_data"
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.

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@yaugenst-flex yaugenst-flex force-pushed the chore/remove-duplicates branch from 8e69994 to ee520f9 Compare October 7, 2025 10:38
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2025

Diff Coverage

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

  • tidy3d/web/api/autograd/backward.py (100%)

Summary

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

@yaugenst-flex yaugenst-flex disabled auto-merge October 7, 2025 10:58
@yaugenst-flex yaugenst-flex force-pushed the chore/remove-duplicates branch 2 times, most recently from 41e91a7 to c367f7f Compare October 7, 2025 10:58
@tylerflex tylerflex self-requested a review October 7, 2025 19:53
yaugenst-flex and others added 2 commits October 8, 2025 09:32
refactor: remove obsolete autograd shims

chore: drop unused autograd wrappers
@yaugenst-flex yaugenst-flex force-pushed the chore/remove-duplicates branch from a98cb28 to 7d7dfb0 Compare October 8, 2025 07:32
@yaugenst-flex yaugenst-flex added this pull request to the merge queue Oct 8, 2025
Merged via the queue into develop with commit bb23c70 Oct 8, 2025
23 checks passed
@yaugenst-flex yaugenst-flex deleted the chore/remove-duplicates branch October 8, 2025 08:51
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.

4 participants