search

efabless / globalfoundries-pdk-libs-gf180mcu_fd_pr

Primitives for GF180MCU provided by GlobalFoundries.
https://gf180mcu-pdk.rtfd.io
Apache License 2.0
9 stars 8 forks source link

document pcells implicit dependency on gdsfactory #28

Open proppy opened 1 year ago

proppy commented 1 year ago

When starting klayout, the following error is raised when gdsfactory is not installed causing the pcells not to load:

ERROR: /home/proppy/silicon-2-env/share/pdk/gf180mcuC/libs.tech/klayout/tech/pymacros/cells/draw_fet.py:22: No module named 'gdsfactory'
  /home/proppy/silicon-2-env/share/pdk/gf180mcuC/libs.tech/klayout/tech/pymacros/cells/draw_fet.py:22
  /home/proppy/silicon-2-env/share/pdk/gf180mcuC/libs.tech/klayout/tech/pymacros/cells/fet.py:19
  /home/proppy/silicon-2-env/share/pdk/gf180mcuC/libs.tech/klayout/tech/pymacros/cells/__init__.py:21
  /home/proppy/silicon-2-env/share/pdk/gf180mcuC/libs.tech/klayout/tech/pymacros/gf180mcu.lym:9 (class ModuleNotFoundError)

This effectively means that the PDK is now dependend on gdsfactory, we should make sure that this is explicitly documented (as most people won't have it installed by default when using klayout and/or volare).

My feelings is that we'd prefer the base klayout integration not to depend on gdsfactory, and have a separate gdsfactory based PDK similar to what @joamatab did with https://github.com/gdsfactory/skywater130/.

curious what @mithro and @joamatab thinks?

atorkmabrains commented 1 year ago

@proppy As discussed here, we will go back to the pya based implementation and remove dependency of gdsfactory for the pcells. I'll move the gdsfactory to another folder outside of klayout to be part of the PR repo. Also, I take the action to add multiple issues/enhancement requests to klayout as agreed above.

Thanks @proppy and @klayoutmatthias Appreciate all the help.

klayoutmatthias commented 1 year ago

I'm sure @klayoutmatthias would love to get a feature request filed that document the missing functionalities in the pya API surface. We can all help adding those primitive to klayout python API and make its ecosystem better.

@proppy Well ... "love to" may not be the right wording in that context, but I am ready to take the callenge :) Please go ahead and file a ticket!

proppy commented 1 year ago

@proppy Well ... "love to" may not be the right wording in that context

Sorry about speaking on your behalf (I should have used würde or hätte here!)

I'll move the gdsfactory to another folder outside of klayout to be part of the PR repo.

What about moving it to a separate repo under the gdsfactory org similar to https://github.com/gdsfactory/skywater130/? /cc @joamatab

we will go back to the pya based implementation

Thanks for considering it.

I thought about another issue that might arise with the current approach: For a given klayout build, we have no guarantee (apart maybe for version properly packaged by linux distribution like debian) that the embedded python version that's linked to klayout is the same as the system python version. Given that the dependency tree of gdsfactory include native dependencies (like matplotlib, numpy, scipy or gdstk) we have no guarantee that if users python -m pip install gdsfactory, they 'll do it from a python environment that's compatible with the one linked to klayout.

atorkmabrains commented 1 year ago

@joamatab Could you please create a repo to move the gdsfactory implementation to? Also, please help us make it a python package installable just like skywaters.

atorkmabrains commented 1 year ago

@proppy BTW, Skywaters klayout support in the open_pdks still use gdsfactory: https://github.com/efabless/sky130_klayout_pdk/blob/main/sky130_tech/tech/sky130/pymacros/cells/draw_fet.py

Is that something you want changed as well?

cc @mkkassem @jeffdi @RTimothyEdwards

proppy commented 1 year ago

BTW, Skywaters klayout support in the open_pdks still use gdsfactory

is that different from https://github.com/gdsfactory/skywater130/, can we consolidate there and revert back to klayout based cells (for the same reason pointed out in https://github.com/efabless/globalfoundries-pdk-libs-gf180mcu_fd_pr/issues/28#issuecomment-1455264196 and https://github.com/efabless/globalfoundries-pdk-libs-gf180mcu_fd_pr/issues/28#issuecomment-1451582312)

/cc @mithro

atorkmabrains commented 1 year ago

https://github.com/gdsfactory/skywater130/ is an older version of the same pcells. I have filed an issue on that repo to update to the latest implementation: https://github.com/gdsfactory/skywater130/issues/63

BTW, we never had pya based pcells for klayout on skywaters like GF180MCU.

proppy commented 1 year ago

BTW, we never had pya based pcells for klayout on skywaters like GF180MCU

what about https://github.com/efabless/sky130_klayout_pdk/blob/14d268fac4df495b3d96e3d8f6058023d99d44c5/sky130_tech/tech/sky130/pymacros/sky130_pcells/imported_generators/nmos18.py ?

atorkmabrains commented 1 year ago

Those are corrupted and should be removed. Thanks for pointing this out. @proppy

atorkmabrains commented 1 year ago

Or in other words, not ready for production. @proppy

joamatab commented 1 year ago

See https://github.com/gdsfactory/gf180

still needs some work, PRs are welcome, see

https://github.com/gdsfactory/gf180/issues/4

atorkmabrains commented 1 year ago

@proppy @klayoutmatthias Klayout issue has been opened: https://github.com/KLayout/klayout/issues/1336

atorkmabrains commented 1 year ago

@proppy and @joamatab Also, I have created an issue on gdsfactory which I think might help here as well: https://github.com/gdsfactory/gdsfactory/issues/1536