Skip to content

Move component metadata to @gf.cell() decorator kwargs#116

Closed
pepijndevos wants to merge 2 commits intogdsfactory:mainfrom
pepijndevos:metadata-annotations
Closed

Move component metadata to @gf.cell() decorator kwargs#116
pepijndevos wants to merge 2 commits intogdsfactory:mainfrom
pepijndevos:metadata-annotations

Conversation

@pepijndevos
Copy link
Copy Markdown
Collaborator

@pepijndevos pepijndevos commented Mar 19, 2026

Summary

  • Move component metadata (symbol, ports, models, tags) from runtime c.info["vlsir"] blocks to static @gf.cell() decorator kwargs, enabling extraction via AST parsing without executing Python
  • Convert 20 PDK cells across 8 files: FET transistors (4), RF transistors (4), resistors (3), capacitors (2), BJTs (4), passives (4), antennas (2), bondpads (1)
  • Fix rppd port_order bug: ["1", "3", "bn"]["1", "2", "bn"] (matching actual .subckt rppd 1 2 bn)
  • Use corner files with .LIB sections (e.g. cornerMOSlv.lib with mos_tt, mos_ss, etc.) instead of model files directly
  • Remove unused model parameter from function signatures
  • Add metadata_guide.md documenting conventions

Test plan

  • Verify @gf.cell() accepts unknown kwargs without error (gdsfactory passes them through as factory_kwargs)
  • Spot-check that cell instantiation still works (e.g. nmos(), rsil(), npn13G2())
  • Verify gfp2-server can extract the new tags, symbol, ports, models from factory_kwargs

🤖 Generated with Claude Code

Summary by Sourcery

Move IHP PDK device metadata from runtime component info to static @gf.cell decorator annotations for AST-based extraction.

New Features:

  • Annotate MOS, RF MOS, BJT, passive, capacitor, antenna, and bondpad cells with tags, symbol layout, ports, and SPICE model metadata via @gf.cell decorator kwargs.
  • Introduce a metadata_guide.md documenting conventions for tags, ports, and model definitions in cell decorators.

Bug Fixes:

  • Correct the rppd resistor SPICE port ordering to match its .subckt definition.

Enhancements:

  • Remove unused model parameters from various PDK cell function signatures and associated runtime metadata assignments.
  • Standardize SPICE model references to use corner .lib files with section names instead of direct model files.

…kwargs

Enables static extraction of component metadata (symbol, ports, models,
tags) by gfp2-server via AST parsing, without executing Python.

- Add tags, symbol, ports, models kwargs to @gf.cell() for 20 PDK cells
- Remove c.info["vlsir"] runtime metadata blocks
- Remove unused `model` parameter from function signatures
- Use corner files (cornerMOSlv.lib, etc.) with .LIB sections instead of
  model files directly
- Fix rppd port_order bug: ["1", "3", "bn"] -> ["1", "2", "bn"]
- Add metadata_guide.md documenting conventions

Converted: fet_transistors (4), rf_transistors (4), resistors (3),
capacitors (2), bjt_transistors (4), passives (4), antennas (2),
bondpads (1). Primitives and fixed.py left as-is (generic SPICE
elements and deprecated wrappers respectively).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Mar 19, 2026

Reviewer's Guide

Refactors IHP PDK device cells so that simulation metadata (tags, symbol, ports, models) is declared statically in @gf.cell decorator kwargs instead of runtime c.info['vlsir'] blocks, while also cleaning up unused model parameters, fixing a resistor port_order bug, standardizing on corner .LIB files, and documenting the new metadata conventions.

Sequence diagram for metadata extraction from gf_cell decorator

sequenceDiagram
    participant GFP2Server
    participant SourceRepo
    participant ASTParser
    participant PythonFile

    GFP2Server->>SourceRepo: list_python_files()
    SourceRepo-->>GFP2Server: paths_to_cell_modules

    loop for_each_cell_module
        GFP2Server->>SourceRepo: read_file(path)
        SourceRepo-->>GFP2Server: file_contents
        GFP2Server->>ASTParser: parse(file_contents)
        ASTParser->>PythonFile: build_ast()
        PythonFile-->>ASTParser: ast_root
        ASTParser->>ASTParser: find_functions_with_decorator(gf.cell)
        ASTParser->>ASTParser: read_decorator_kwargs(tags, symbol, ports, models)
        ASTParser-->>GFP2Server: CellMetadata(tags, symbol, ports, models)
    end

    GFP2Server->>GFP2Server: build_device_catalog_from_metadata()
Loading

Class diagram for gf_cell decorator metadata structure

classDiagram
    class GfCellDecorator {
        +list~string~ tags
        +string symbol
        +Ports ports
        +list~ModelSpec~ models
    }

    class Ports {
        +list~string~ top
        +list~string~ bottom
        +list~string~ left
        +list~string~ right
    }

    class ModelSpec {
        +string language
        +string name
        +string spice_type
        +string library
        +list~string~ sections
        +list~string~ port_order
        +dict~string,string~ params
    }

    class ComponentFactory {
        +Component __call__(kwargs)
        +dict factory_kwargs
    }

    class Component {
        +dict info
        +dict ports
        +add_label(text, position, layer)
        +add_port(name, center, width, orientation, layer, port_type)
    }

    GfCellDecorator --> Ports : uses
    GfCellDecorator --> ModelSpec : uses
    GfCellDecorator --> ComponentFactory : wraps
    ComponentFactory --> Component : creates
    Component --> ModelSpec : metadata_via_factory_kwargs
Loading

File-Level Changes

Change Details Files
Move simulation metadata from runtime c.info['vlsir'] dicts into static @gf.cell decorator kwargs for all affected IHP device cells.
  • Add tags, symbol, ports, and models keyword arguments to @gf.cell for MOS, RF MOS, BJT, passive, capacitor, antenna, and bondpad cells
  • Encode model information as a list of dicts including language, name, spice_type, library, sections, port_order, and params with string expressions over function arguments
  • Remove all c.info['vlsir'] assignments from the converted cells so metadata is no longer stored at runtime
ihp/cells/fet_transistors.py
ihp/cells/rf_transistors.py
ihp/cells/bjt_transistors.py
ihp/cells/passives.py
ihp/cells/resistors.py
ihp/cells/capacitors.py
ihp/cells/antennas.py
ihp/cells/bondpads.py
Simplify cell function APIs by dropping unused model parameters and adjusting related metadata handling.
  • Remove the model parameter from public cell function signatures where it was never used in the body
  • Stop storing a model entry in c.info where it previously mirrored the removed parameter or hardcoded name
  • Update internal uses (e.g., labels, CbCapCalc calls) to use explicit model names instead of relying on the removed parameter
ihp/cells/fet_transistors.py
ihp/cells/rf_transistors.py
ihp/cells/passives.py
ihp/cells/resistors.py
ihp/cells/capacitors.py
Standardize SPICE model referencing on corner .LIB files and correct port ordering inconsistencies.
  • Change model metadata to reference corner libraries (e.g. cornerMOSlv.lib, cornerMOShv.lib, cornerRES.lib, cornerCAP.lib, cornerDIO.lib, cornerHBT.lib) instead of direct model files
  • Ensure sections lists in model metadata correspond to PVT corner names from the corner libraries
  • Fix rppd resistor model port_order from ["1", "3", "bn"] to ["1", "2", "bn"] to match the .subckt definition and align port mapping across resistors
ihp/cells/fet_transistors.py
ihp/cells/rf_transistors.py
ihp/cells/bjt_transistors.py
ihp/cells/passives.py
ihp/cells/resistors.py
ihp/cells/capacitors.py
ihp/cells/antennas.py
ihp/cells/bondpads.py
Adjust per-cell metadata usage and naming to be consistent with the new decorator-based model definitions.
  • For cmim and rfcmim, hardcode the model name cap_cmim and cap_rfcmim where previously a model parameter was passed through, including labels and CbCapCalc calls
  • Align port naming and symbol orientation metadata across devices (e.g., MOSFET orientation D top, S bottom, G left, B right; BJTs with C/B/E/S mapping)
  • Preserve or minimally adjust non-vlsir c.info fields (like geometry, rows/cols, capacitance) while removing only the simulation-specific entries
ihp/cells/bjt_transistors.py
ihp/cells/passives.py
ihp/cells/resistors.py
ihp/cells/capacitors.py
ihp/cells/antennas.py
ihp/cells/bondpads.py
Document the new metadata conventions for future cell conversions and static extraction tooling.
  • Add metadata_guide.md describing conventions for tags, ports, model library/sections, port_order, and params expression strings
  • Explain the rationale for moving metadata into the decorator to enable AST-based extraction without executing Python
  • Clarify that c.info['vlsir'] and the model parameter are deprecated/removed in favor of decorator kwargs
metadata_guide.md

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 found 1 issue, and left some high level feedback:

  • Several model definitions (e.g., LV/HV MOS and RF variants, poly resistors) repeat nearly identical models blocks in the decorators; consider factoring these into shared helpers or constants to avoid drift if a library name, corner list, or parameter expression needs to change later.
  • Where the old c.info['vlsir'] metadata carried additional non-geometric parameters (e.g., bondpad size/shape/padtype, resistor m, ESD ng), consider adding equivalent entries into the new models[].params dictionaries so downstream consumers don’t lose information that was previously available.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Several model definitions (e.g., LV/HV MOS and RF variants, poly resistors) repeat nearly identical `models` blocks in the decorators; consider factoring these into shared helpers or constants to avoid drift if a library name, corner list, or parameter expression needs to change later.
- Where the old `c.info['vlsir']` metadata carried additional non-geometric parameters (e.g., bondpad `size`/`shape`/`padtype`, resistor `m`, ESD `ng`), consider adding equivalent entries into the new `models[].params` dictionaries so downstream consumers don’t lose information that was previously available.

## Individual Comments

### Comment 1
<location path="ihp/cells/capacitors.py" line_range="651-652" />
<code_context>
         layer_topmetal1label=layer_topmetal1label,
-        model=model,
     )
     c.info = cap.info
     c.add_ref(cap)
</code_context>
<issue_to_address>
**issue (bug_risk):** rfcmim `c.info["model"]` will still be `"cap_cmim"` instead of `"cap_rfcmim"` after copying from the inner cmim.

Because `c.info = cap.info` copies the inner `cmim` metadata (where `c.info["model"] = "cap_cmim"`), `rfcmim` will advertise the wrong model name. Please reset `c.info["model"]` to `"cap_rfcmim"` after copying to keep the metadata accurate for tools that read `c.info["model"]`.
</issue_to_address>

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.

Comment thread ihp/cells/capacitors.py
c.info = cap.info copies cmim's info dict, so model was "cap_cmim"
instead of "cap_rfcmim". Reset it after copying. Also update
metadata_guide.md with a note about wrapper cells.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ThomasPluck ThomasPluck self-requested a review March 19, 2026 13:19
Copy link
Copy Markdown
Contributor

@ThomasPluck ThomasPluck left a comment

Choose a reason for hiding this comment

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

Okay in order for this, we need to fix gdsfactory upstream to allow for arb @gf.cell decorators (issue: gdsfactory/gdsfactory#4452) - let's track this and merge with the latest version.

@pepijndevos pepijndevos marked this pull request as draft March 19, 2026 13:33
@pepijndevos
Copy link
Copy Markdown
Collaborator Author

Yes makes sense I've made it a draft because this needs testing on all sides

@pepijndevos
Copy link
Copy Markdown
Collaborator Author

oh, we should also discover the sax model and add it as an explicit link

@ThomasPluck
Copy link
Copy Markdown
Contributor

See gdsfactory/gdsfactory#4455

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.

2 participants