Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glayout PrePEX SPICE Generation #276

Merged
merged 57 commits into from
Nov 19, 2023

Conversation

harshkhandeparkar
Copy link
Collaborator

@harshkhandeparkar harshkhandeparkar commented Nov 17, 2023

Changes

  • Added Netlist class for SPICE generation in glayout/spice/.
  • Added SPICE models to the MappedPDK class.
  • Added prePEX SPICE netlists for primitive cells, diff pair, differential_to_single_ended_converter.
  • Added complete prePEX SPICE generation for the glayout op-amp.

Copy link
Collaborator

@alibillalhammoud alibillalhammoud left a comment

Choose a reason for hiding this comment

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

@harshkhandeparkar see changes when you get the chance. Mainly focus on keeping Netlist separate from layout. This is for easier debug in the future and for better code organization. Also make deepcopys within the Netlist class rather than having users decide when to deepcopy.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of doing a deepcopys for your get_netlist function. Wrap everything within the Netlist class. This is better for people using the API (most people do not think to do a deep copy and the API should take care of that behind the scenes). If the get_netlist function takes a dictionary everytime, you can just deepcopy by default (probably easiest thing to do here).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The deepcopy cannot be done internally since the netlist can be referred to externally in the connect_subnets or other functions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pass the shallow copy and on the first line of get_netlist or connect_netlist perform a deepcopy

Copy link
Collaborator

Choose a reason for hiding this comment

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

One function should not return both Netlist and Component. Add another function which makes a Netlist, and always add the Netlist directly to the component in the Component.info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, I need the reference to the NMOS, amp_fet_ref, since its netlist is required for generating the output stage netlist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I refactored the rest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you need the netlist put it into the component itself. When you return the component you will have the netlist

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This component already has a netlist

Copy link
Collaborator

Choose a reason for hiding this comment

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

same issue (one function should not return netlist and Component)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have refactored most of it but I still need a reference to the mimcap netlist.

Copy link
Collaborator

Choose a reason for hiding this comment

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

put it into the component.info and only return the component

Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed above, where ever applicable make deepcopys of inputs so that the whoever uses this class does not need to.

Copy link
Collaborator

@alibillalhammoud alibillalhammoud left a comment

Choose a reason for hiding this comment

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

@harshkhandeparkar see changes when you get the chance. Mainly focus on keeping Netlist separate from layout. This is for easier debug in the future and for better code organization. Also make deepcopys within the Netlist class rather than having users decide when to deepcopy.

Copy link
Collaborator

@alibillalhammoud alibillalhammoud left a comment

Choose a reason for hiding this comment

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

Much better. just deepcopy on the first line of connect_netlist or get_netlist and you should be fine to avoid it everywhere else

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pass the shallow copy and on the first line of get_netlist or connect_netlist perform a deepcopy

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you need the netlist put it into the component itself. When you return the component you will have the netlist

Copy link
Collaborator

Choose a reason for hiding this comment

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

put it into the component.info and only return the component

Copy link
Collaborator

@alibillalhammoud alibillalhammoud left a comment

Choose a reason for hiding this comment

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

implemented changes, nice job Harsh

@@ -35,7 +35,42 @@
from glayout.pdk.util.snap_to_grid import component_snap_to_grid
Copy link
Member

Choose a reason for hiding this comment

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

@harshkhandeparkar Can you add a description

@msaligane msaligane merged commit 1a3607d into idea-fasoc:main Nov 19, 2023
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