Skip to content

Fix: cascaded mzi notebook + tidy3d pluggin bug fixes#685

Open
David-GERARD wants to merge 4 commits intogdsfactory:mainfrom
David-GERARD:fix/cascaded-mzi-notebook
Open

Fix: cascaded mzi notebook + tidy3d pluggin bug fixes#685
David-GERARD wants to merge 4 commits intogdsfactory:mainfrom
David-GERARD:fix/cascaded-mzi-notebook

Conversation

@David-GERARD
Copy link
Copy Markdown
Contributor

@David-GERARD David-GERARD commented Mar 26, 2026

Overview

The end of this notebook is commented-out as meant to have a bug. I took a look at it, and in the process founds more errors.

Currently, the Notebook runs successfully IF AND ONLY IF the test data is downloaded trough make test-data. Let me know if this is enough to merge.

Changelog

  • Bumped the required version of gdsfactory to be able to run gf.gpdk.PDK.activate() without error. 97a5d69

Future work

  • Update the Notebook in the docs/ folder and veryfy that the documentation builds.
  • tidy3d.wed.run() has been changed from the version the pluggins expects. In version 2.8.5 (current requirements is "tidy3d>=2.8.2,<2.9"), it expects a mandatory task name. However, it seems that in newer versions this is no longer the case.
  • I flagged another bug in commit b05d6e4 as web.run() no longer accepts an object of the ComponentModeler class.

In general, I think that the tidy3d pluggin might need some work that goes beyon just fixing this Notebook.

Summary by Sourcery

Fix the cascaded MZI workflow notebook to run end-to-end and align the tidy3d plugin with updated API requirements.

Bug Fixes:

  • Enable generation and loading of S-parameters in the cascaded MZI notebook using batch simulation jobs.
  • Pass a stable task name and correct output path when running tidy3d web simulations to restore S-parameter computation.

Enhancements:

  • Update the cascaded MZI notebook kernel metadata to use the project’s gplugins environment.

Build:

  • Bump the minimum gdsfactory version requirement to 9.29.1 to support the current PDK activation API.

@David-GERARD David-GERARD requested a review from joamatab as a code owner March 26, 2026 09:30
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai bot commented Mar 26, 2026

Reviewer's Guide

Updates the cascaded MZI workflow notebook to compute and batch-run s‑parameter simulations programmatically, aligns the Tidy3D plugin with newer web.run semantics using hashed task names and output directories, and bumps the gdsfactory dependency version required to activate the PDK without errors.

Sequence diagram for updated write_sparameters and web.run interaction

sequenceDiagram
    participant User
    participant write_sparameters
    participant ComponentModeler as ComponentModeler
    participant web
    participant Filesystem

    User->>write_sparameters: call write_sparameters(component, dirpath, ...)
    write_sparameters->>ComponentModeler: create modeler from component and settings
    write_sparameters->>ComponentModeler: compute hash via _hash_self()
    ComponentModeler-->>write_sparameters: task_name
    write_sparameters->>Filesystem: build path_dir = pathlib.Path(dirpath) / task_name
    write_sparameters->>ComponentModeler: modeler = modeler.updated_copy()
    write_sparameters->>Filesystem: check if cached sparameters file exists
    alt cached sparameters exists
        write_sparameters->>Filesystem: load npz file
        Filesystem-->>write_sparameters: sparameters data
        write_sparameters-->>User: return cached sparameters
    else no cache
        write_sparameters->>web: run(modeler, task_name=task_name, verbose=verbose, path=path_dir / simulation.hdf5)
        web-->>write_sparameters: modeler_data
        write_sparameters->>write_sparameters: compute s = modeler_data.smatrix()
        write_sparameters->>Filesystem: save sparameters to filepath
        write_sparameters-->>User: return computed sparameters
    end
Loading

Flow diagram for updated cascaded MZI notebook sparameter batch simulation

flowchart TD
    A_start[Start notebook] --> B_defineSimLengths[Define sim_lengths and other parameters]
    B_defineSimLengths --> C_buildJobs[Build jobs list for each length in sim_lengths]
    C_buildJobs -->|for each length| C1_jobItem[Create dict with component, filepath, layer_stack]
    C_buildJobs --> D_callBatch[Call gt.write_sparameters_batch with jobs]
    D_callBatch --> E_getFutures[Receive list of simulation futures sims]
    E_getFutures --> F_collectResults[Call result on each future to build s_params_list]
    F_collectResults --> G_postProcess[Compute wavelengths, drop, through, error metrics]
    G_postProcess --> H_plot[Plot coupler responses and fitting errors]
    H_plot --> I_continueNotebook[Continue with filter modeling and full system simulation]
    I_continueNotebook --> J_end[End notebook]
Loading

File-Level Changes

Change Details Files
Fix cascaded MZI notebook to generate and consume s-parameter simulations via write_sparameters_batch instead of relying on precomputed files.
  • Remove the commented-out construction of s_params_list from pre-saved NPZ files and instead build it from simulation results.
  • Create a jobs list of dicts (component, filepath, layer_stack) over sim_lengths and pass it to gt.write_sparameters_batch.
  • Update the notebook’s kernel metadata to use the gplugins (3.11.14) environment and Python 3.11.14.
notebooks/workflow_3_cascaded_mzi.ipynb
Adjust Tidy3D component s-parameter writer to work with newer web.run API requirements, including explicit task naming and per-task directories.
  • Derive a task_name from modeler._hash_self() and use it to form the per-simulation output directory.
  • Pass task_name and an updated path pointing into the task-specific directory to web.run so simulations have unique names and output files.
  • Leave a TODO noting that web.run currently does not support ComponentModeler objects directly and will require conversion to the appropriate simulation type.
gplugins/tidy3d/component.py
Increase the minimum gdsfactory version to ensure PDK activation and compatibility with updated APIs.
  • Bump gdsfactory dependency from >=9.15.1 to >=9.29.1 in pyproject.toml.
  • Regenerate uv.lock to reflect the new dependency graph.
pyproject.toml
uv.lock

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The notebook metadata now hardcodes a local kernel name ("display_name": "gplugins (3.11.14)" and Python 3.11.14); consider reverting those fields so the notebook remains environment-agnostic for other users.
  • In write_sparameters, the web.run call now includes a TODO about ComponentModeler not being supported; it may be better to either implement a minimal conversion or explicitly raise a clear error when a ComponentModeler instance is passed instead of relying on a comment that can be overlooked.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The notebook metadata now hardcodes a local kernel name (`"display_name": "gplugins (3.11.14)"` and Python 3.11.14); consider reverting those fields so the notebook remains environment-agnostic for other users.
- In `write_sparameters`, the `web.run` call now includes a TODO about `ComponentModeler` not being supported; it may be better to either implement a minimal conversion or explicitly raise a clear error when a `ComponentModeler` instance is passed instead of relying on a comment that can be overlooked.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

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.

1 participant