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

Update skywater130 pcells implementation with the its latest version #67

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

Conversation

RehabSayed-G
Copy link
Contributor

No description provided.

@atorkmabrains
Copy link
Contributor

@joamatab I'm not sure why the pre-commit fails

@joamatab
Copy link
Contributor

joamatab commented Apr 6, 2023

i think you need to run

pre-commit run -a   

and push again

@atorkmabrains
Copy link
Contributor

@RehabSayed-G Could you please run the command highlighted above?

@RehabSayed-G
Copy link
Contributor Author

@joamatab ,

  • the pre-commit fails as files are not clean (black and flake8)
  • it gives this error when running : pre-commit run -a
    image

@atorkmabrains
Copy link
Contributor

@joamatab Appreciate if you check why this is happening?

@atorkmabrains
Copy link
Contributor

@joamatab Could you please approve the change?

@atorkmabrains
Copy link
Contributor

@joamatab Could you please review and merge this?

@joamatab
Copy link
Contributor

Hi Amro, sorry i missed this, I think there is some license headers left on the code, how about removing those as the repo already has a license file?

@joamatab
Copy link
Contributor

Also, you are using klayout strange python syntax to make them,

Could we use kfactory that has a syntax similar to gdsfactory instead?

@proppy
@sebastian-goeldi

@sebastian-goeldi
Copy link

This won't work in kfactory (yet and at least for the foreseeable future), exactly because of the "strange" classes. These classes are necessary for the klayout PCell implementation.

The pcells are required to be class instances instead of mere functions like in gdsfactory/kfactory. Sure we could use the same workaround like flayout used which essentially boiled down to write gds and load during the pcell implementation, but it's everything but a good solution (esp if you run into cell name conflicts).

I can check whether we can find an okay solution, but on windows (and to some degree on MacOS) it's not fun unless we get klayout in a conda environment where we can control the packages installed.

@joamatab
Copy link
Contributor

joamatab commented Jan 16, 2024

How about using something like flayout that Floris made to automatically translate gdsfactory Pcells into Klayout Pcells?

Floris made a flayout script to do all this automatically without having to rewrite the Pcell in Klayout strange PCell syntax

https://github.com/flaport/flayout

@flaport
@klayoutmatthias
@thomaslima

@atorkmabrains
Copy link
Contributor

@joamatab There is no need to translate anything, all code was implemented via gdsfactory. Only the GUI interface was implemented for klayout. Here is an example:
https://github.com/mabrains/skywater130/blob/9d7ecf1deeadf09708172e7a0a3f0d3f4381f81f/sky130/cells/klayout/pymacros/cells/fet.py#L38

The above is just the declaration of the pfet for klayout.

GDS factory cell:
https://github.com/mabrains/skywater130/blob/main/sky130/cells/klayout/pymacros/cells/draw_fet.py#L936

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.

4 participants