Skip to content

Conversation

marcorudolphflex
Copy link
Contributor

@marcorudolphflex marcorudolphflex commented Oct 9, 2025

Goal
Extend web.run (and only the surfaces that already wrap it - autograd entry points, CLI helpers, Job/Batch, run_async) so the API accepts arbitrary Python containers and returns SimulationData lazily in the same structure, relying on the lazy-load.

Implementation

  • web.run now delegates to legacy run for single runs and to run_async for multiple runs
  • input is parsed for simulation type objects, flattened into a dict with sim hashes as keys, used as batch in run_async and then rearranged into the input structure
  • path is interpreted as dir for multiple runs and filename for single runs

Open questions:
If same simulation is submitted multiple times, the sim_data objects of these simulations are the same - should we make copies?

Greptile Overview

Updated On: 2025-10-10 15:52:48 UTC

Greptile Summary

This PR implements a major enhancement to the web.run() API, making it container-aware so it can accept arbitrary Python containers (lists, tuples, dictionaries) containing simulation objects and return results in the same nested structure. The implementation introduces a unified interface that maintains backward compatibility while adding powerful batch processing capabilities.

The core change is a new tidy3d/web/api/run.py module that serves as a container-aware wrapper. This wrapper intelligently delegates to the existing legacy run function for single simulations and to run_async for multiple simulations. The implementation uses hash-based deduplication to avoid running identical simulations multiple times, optimizing performance when the same simulation appears in different parts of the input structure.

A new lazy parameter has been added throughout the web API stack, allowing users to control whether simulation data is loaded immediately (eager) or deferred until first access (lazy). The lazy loading behavior defaults to False for single simulations (maintaining existing behavior) and True for batch runs (more efficient for large operations).

The changes extend across multiple layers including the main web interface, autograd entry points, container classes (Job, Batch, BatchData), and asynchronous operations. Path parameter interpretation has also been enhanced to treat paths as directories for multiple runs and filenames for single runs, providing intuitive behavior based on input type.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/web/api/run.py 3/5 New container-aware wrapper with hash-based deduplication but contains debugging print statements that must be removed
tidy3d/web/api/container.py 4/5 Added lazy loading support to Job, Batch, and BatchData classes with proper parameter propagation
tests/test_web/test_webapi.py 4/5 Added comprehensive tests for container-aware functionality with good coverage
tidy3d/web/api/webapi.py 5/5 Added lazy parameter to run function with proper documentation and implementation
tidy3d/web/api/asynchronous.py 5/5 Added lazy parameter support to run_async function with proper forwarding
tidy3d/web/api/autograd/autograd.py 5/5 Added lazy parameter support to autograd run functions maintaining compatibility
tidy3d/web/api/autograd/engine.py 5/5 Added lazy parameter to job_fields for autograd compatibility
tidy3d/web/__init__.py 5/5 Updated imports to use new container-aware run implementation
tests/test_web/test_webapi_mode.py 5/5 Updated imports to reflect new run module location
CHANGELOG.md 5/5 Added changelog entry documenting the new container-aware web.run API
tidy3d/plugins/smatrix/run.py 4/5 Refactored to use new container-aware run_async API with proper separation of concerns
tidy3d/packaging.py 5/5 Minor cleanup of error message text for better user experience

Confidence score: 3/5

  • This PR introduces significant functionality but contains debugging code that must be removed before merging
  • Score reflects the presence of print statements and complex container traversal logic requiring careful review
  • Pay close attention to tidy3d/web/api/run.py which contains debugging print statements that violate coding standards

Sequence Diagram

sequenceDiagram
    participant User
    participant run_func as "run()"
    participant collect_by_hash as "_collect_by_hash"
    participant run_autograd as "run_autograd"
    participant run_async as "run_async"
    participant reconstruct_by_hash as "_reconstruct_by_hash"

    User->>run_func: "call run() with simulation container"
    run_func->>collect_by_hash: "extract simulations by hash"
    collect_by_hash-->>run_func: "return {hash: sim} mapping"
    
    alt Single simulation
        run_func->>run_autograd: "run single simulation"
        run_autograd-->>run_func: "return simulation data"
    else Multiple simulations
        run_func->>run_async: "run batch of simulations"
        run_async-->>run_func: "return batch data dict"
    end
    
    run_func->>reconstruct_by_hash: "rebuild container structure"
    reconstruct_by_hash-->>run_func: "return structured results"
    run_func-->>User: "return results in original container shape"
Loading

Context used:

  • Rule from dashboard - Remove temporary debugging code (print() calls), commented-out code, and other workarounds before fi... (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.

10 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@marcorudolphflex marcorudolphflex force-pushed the FXC-3004-container-aware-web-run-returning-lazy-results branch from c9a5088 to 5217204 Compare October 9, 2025 10:23
@flexcompute flexcompute deleted a comment from greptile-apps bot Oct 9, 2025
@marcorudolphflex marcorudolphflex force-pushed the FXC-3004-container-aware-web-run-returning-lazy-results branch from 5217204 to 145debb Compare October 9, 2025 10:26
Copy link
Contributor

github-actions bot commented Oct 9, 2025

Diff Coverage

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

  • tidy3d/web/init.py (100%)
  • tidy3d/web/api/autograd/engine.py (100%)
  • tidy3d/web/api/container.py (100%)
  • tidy3d/web/api/run.py (92.5%): Missing lines 46,50,76,91,236

Summary

  • Total: 76 lines
  • Missing: 5 lines
  • Coverage: 93%

tidy3d/web/api/run.py

Lines 42-54

  42             _collect_by_hash(v, found)
  43         return found
  44     if isinstance(node, dict):
  45         if any(isinstance(k, WorkflowType) for k in node.keys()):
! 46             raise ValueError("Dict keys must not be simulations.")
  47         for v in node.values():
  48             _collect_by_hash(v, found)
  49         return found
! 50     raise TypeError(f"Unsupported element in container: {type(node)!r}")
  51 
  52 
  53 def _reconstruct_by_hash(node: RunInput, h2data: dict[str, WorkflowDataType]) -> RunOutput:
  54     """Replaces each leaf node (simulation) with its corresponding data object.

Lines 72-80

  72         if isinstance(n, list):
  73             return [_recur(v) for v in n]
  74         if isinstance(n, dict):
  75             return {k: _recur(v) for k, v in n.items()}
! 76         raise TypeError(f"Unsupported element in reconstruction: {type(n)!r}")
  77 
  78     return _recur(node)
  79 

Lines 87-95

  87     """
  88     has_modeler = any(isinstance(sim, ComponentModelerType) for sim in simulations)
  89 
  90     if has_modeler and len(simulations) > 1:
! 91         raise ValueError(
  92             "web.run() does not support ComponentModelerTypes when submitting multiple simulations."
  93         )
  94 

Lines 232-240

  232         Underlying autograd batch submission implementation.
  233     """
  234     h2sim: dict[str, WorkflowType] = _collect_by_hash(simulation)
  235     if not h2sim:
! 236         raise ValueError("No simulation data found in simulation input.")
  237 
  238     _check_simulation_types(list(h2sim.values()))
  239 
  240     key_prefix = ""

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 @marcorudolphflex this is great! I left a couple of comments, nothing too major.

@marcorudolphflex marcorudolphflex force-pushed the FXC-3004-container-aware-web-run-returning-lazy-results branch from 145debb to e03175a Compare October 10, 2025 15:48
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.

Additional Comments (1)

  1. tidy3d/plugins/smatrix/run.py, line 160-181 (link)

    syntax: Docstring describes the old implementation and mentions returning ComponentModelerDataType but the function now returns tuple[SimulationMap, dict[str, Any]]

    Context Used: Rule from dashboard - Keep docstrings and comments synchronized with code changes to ensure they are always accurate and n... (source)

12 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@marcorudolphflex marcorudolphflex force-pushed the FXC-3004-container-aware-web-run-returning-lazy-results branch 2 times, most recently from 7f259d2 to 853a2b0 Compare October 10, 2025 16:04
@marcorudolphflex marcorudolphflex force-pushed the FXC-3004-container-aware-web-run-returning-lazy-results branch from 853a2b0 to 337db93 Compare October 10, 2025 16:14
Copy link
Collaborator

@daquinteroflex daquinteroflex Oct 13, 2025

Choose a reason for hiding this comment

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

So it seems the main issue is maintaining compatibility of the old webapi.run functionality in the new run_async in autograd with the new workflow as they were not directly interchangeable before. Probably we can update some tests to catch this, but I am working on this in my PR

Hmm more things are going on

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