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

C route and other fixes #358

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

Subhampal9
Copy link

  1. added code to add more vias at intermediate nodes of c_route
  2. minor fix in four transistor interdigitized block

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove these files and only keep the files relevant to the fix?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you highlight which lines were changed? Or revert the styling changes so that the diff only shows the relevant changes.

Copy link
Author

Choose a reason for hiding this comment

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

I have added some comments where the changes are

raise ValueError("Ports must be parralel and have same orientation")
width1 = width1 if width1 else edge1.width
width2 = width2 if width2 else edge1.width
cwidth = cwidth if cwidth else max(width1,width2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is cwidth here? It is an argument but there is no description for it in the docstring. Can you add a line for it?

Copy link
Author

Choose a reason for hiding this comment

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

it is the width of the intermediate connection route

if round(edge1.orientation) == 90 or round(edge1.orientation) == 270:
viastack1 = via_array(pdk, e1glayer, cglayer, size=(width1,cwidth), fullbottom=fullbottom, no_exception=True)

viastack2_dims = evaluate_bbox(viastack2,True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code repeats here. Can you use loops or some other method to reduce the duplication?

Copy link
Author

Choose a reason for hiding this comment

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

via_array is taking different sizes in each case, I am not sure how I can loop it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can create a list and iterate over it. It is not required though.

Copy link
Author

Choose a reason for hiding this comment

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

So, should I leave it as it is?

Copy link
Member

Choose a reason for hiding this comment

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

Can you create a list?

viastack2 = via_stack(pdk,e2glayer,cglayer,fullbottom=fullbottom,assume_bottom_via=True,fulltop=True)

viastack1_dims = evaluate_bbox(viastack1,True)
#condition checking for multiple vias at first intermediate node
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also describe the condition here? eg: "Creates a via array if ..."

Copy link
Author

Choose a reason for hiding this comment

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

okay

added comments and updated docstring
Copy link
Member

@msaligane msaligane left a comment

Choose a reason for hiding this comment

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

@Subhampal9 Great work! It would be good to start documenting some of the changes. Can you add some simple readmes for c_route for instance.

@Subhampal9
Copy link
Author

@Subhampal9 Great work! It would be good to start documenting some of the changes. Can you add some simple readmes for c_route for instance.

Will do that soon.

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