gdsfactory-gen generators and DRC parsing #218
Conversation
Co-authored-by: vijayshankarr <94952142+vijayshankarr@users.noreply.github.com>
|
@joamatab Do you think you can help me review this? |
Co-authored-by: Ryan Wans <37909218+ryanrocket@users.noreply.github.com>
|
|
||
|
|
||
| def sky130_add_opamp_labels(opamp_in: Component) -> Component: | ||
| opamp_in.unlock() |
There was a problem hiding this comment.
You can create a reference so you don't have to do this hack
|
To ensure the stability and integrity of our gdsfactory Pcells, it is crucial that we adhere to a specific pattern in order to prevent any unintended modifications. These Components are locked for a reason, and it is important that we refrain from unlocking and modifying them once they are in the cache. For a detailed explanation of the recommended pattern, please refer to the following link:/workflow_0_layout_summary.html?highlight=summary This is the general pattern we follow @gf.cell
def mzi_with_bend(radius: float = 10):
"""Returns MZI interferometer with bend."""
c = gf.Component() # the top level component (our canvas)
mzi = c.add_ref(gf.components.mzi()) # first reference
bend = c.add_ref(gf.components.bend_euler(radius=radius)) # second reference
bend.connect("o1", mzi.ports["o2"]) # connect
c.add_port("o1", port=mzi.ports["o1"]) # expose some ports
c.add_port("o2", port=bend.ports["o2"]) # expose some ports
return coverall i'd recommend adding the skywater130 and gf180 pdks into the gdsfactory generic PDK |
|
@alibillalhammoud can you please mark an order of merging for your PRs? |
|
@alibillalhammoud have you synced your Fork? There are too many changes to review. Let's go over this today. |
|
@saicharan0112 I just have this PR to merge. @msaligane I have synced my fork with main. We can go over today. |
There was a problem hiding this comment.
@alibillalhammoud are these gds files using somewhere else? it looks like you moved them in deprecated.
There was a problem hiding this comment.
Not sure what those are for, I did not modify the "deprecated" directories at all. If we want we can delete the deprecated directories. They will be in the git history anyways
There was a problem hiding this comment.
Would be good to add a couple comments regarding these port trees
There was a problem hiding this comment.
Did we create a github issue to track this. This file should be copied from the pdk folder
There was a problem hiding this comment.
@msaligane see issue 235. This tracks all the changes I need to make.
There was a problem hiding this comment.
can we add a description of this fct.
There was a problem hiding this comment.
Seems like there are no descriptions to your fcts. So it is hard to follow.
openfasoc/generators/gdsfactory-gen/tapeout_and_RL/sky130_mpw5_pad.gds
Outdated
Show resolved
Hide resolved
|
@alibillalhammoud I am not sure if this is part of the plan (I don't see it in the tasks) but try to create a Jupyter notebook on how to work with pygen-openfasoc. I think that's the best documentation someone could start with |
| grulesobj["dnwell"]["dnwell"] = {"min_width": 3.0, "min_separation": 6.3} | ||
| grulesobj["dnwell"]["pwell"] = {"min_enclosure": 0.0} | ||
| grulesobj["dnwell"]["nwell"] = {"min_separation": 4.5} | ||
| grulesobj["dnwell"]["p+s/d"] = {} |
There was a problem hiding this comment.
Can these empty dict initializations be done using two for loops?
There was a problem hiding this comment.
It would be better to just keep the PNG files for these trees in the docs. I would also suggest creating a subdirectory for the images.
There was a problem hiding this comment.
Can we include gdsfactory dependency in this file? The root requirements file locks the gdsfactory version to 5.1.1. The latest version is 7.4.3. Version 5.1.1 doesn't work on my system with Python 3.11.
There was a problem hiding this comment.
@harshkhandeparkar did you make a gdsfactory issue about the broken pip dependencies for the sky130 and gf180 packages?
There was a problem hiding this comment.
I haven't yet, but I will soon. I want to test more to figure out what the exact issue is and make sure it is not a problem with my system.
There was a problem hiding this comment.
I was looking for this exact comment. Is still the old gdsfactory used?. I see that this commit has something about the gdsfactory version in its message.
There was a problem hiding this comment.
Also where exactly the packages mentioned inside this requirements.txt file are installed?
There was a problem hiding this comment.
This requirements.txt file just contains dependencies necessary for the gdsfactory generators. They are not installed separately, just listed in a separate file. But for some reason, the root requirements.txt also has gdsfactory=5.1.1 mentioned as a dependency which it should probably not be.
| All functions, classes, etc have a help docustring. See python help() for specific questions | ||
|
|
||
| - [Pygen](#pygen) | ||
| - [MappedPDK](#mappedpdk) |
There was a problem hiding this comment.
I think it would be good to have a short section on installing dependencies and running the code.
| interfinger_rmult: int=1 | ||
| ) -> Component: | ||
| """Generic NMOS generator | ||
| pdk = mapped pdk to use |
There was a problem hiding this comment.
These parameters are rendered on a single line in VSCode intellisense.
I would suggest using markdown list syntax or : instead of =
- pdk = ...
- width = ...or
- pdk: ...
- width: ...| for glayer in MappedPDK.valid_glayers: | ||
| grulesobj[glayer] = dict((x, None) for x in MappedPDK.valid_glayers) | ||
|
|
||
| grulesobj["dnwell"]["dnwell"] = {"min_width": 3.0, "min_separation": 6.3} |
There was a problem hiding this comment.
Are these values manually entered, or are they pulled from some file? If they are pulled from a file from the PDK, can this process also be automated?
openfasoc/generators/gdsfactory-gen/pygen/pdk/sky130_mapped/sky130_add_npc.py
Outdated
Show resolved
Hide resolved
openfasoc/generators/gdsfactory-gen/pygen/pdk/util/print_rules.py
Outdated
Show resolved
Hide resolved
| via_flush1 = 0 if viaoffset[0] is None else via_flush1 | ||
| via_flush2 = via_flush if viaoffset[1] else 0-via_flush | ||
| via_flush2 = 0 if viaoffset[1] is None else via_flush2 | ||
| if round(edge1.orientation) == 0:# facing east |
There was a problem hiding this comment.
It seems like the code under this function is repeated many times. Is there a possibility for refactoring?
There was a problem hiding this comment.
This is a 1000-line file. I would suggest splitting it for readability.
I think I just saw one of the issues on documentation for gdsfactory-gen that includes python notebook. |
|
So can we merge both requirement files? I unpinned the version number for
gdsfactory btw. Earlier I pinned it because of some python
version/installation issues. Now I don't see that anymore
…On Mon, 2 Oct, 2023, 6:24 pm Harsh Khandeparkar, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
On openfasoc/generators/gdsfactory-gen/requirements.txt
<#218 (comment)>:
This requirements.txt file just contains dependencies necessary for the
gdsfactory generators. They are not installed separately, just listed in a
separate file. But for some reason, the root requirements.txt also has
gdsfactory=5.1.1 mentioned as a dependency which it should probably not
be.
—
Reply to this email directly, view it on GitHub
<#218 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AYCOBKPRVXFQICXN32YCSNTX5K2O3AVCNFSM6AAAAAA2NEWXNWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNJSGY4TEOJYGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This PR provides an intermediate step towards the goals described in issue210. This introduces the following generators and features: