models/{ngspice,xyce}/testing/README: should document how to interpret simulation result about globalfoundries-pdk-libs-gf180mcu_fd_pr HOT 16 OPEN

Hi @proppy and @mkkassem Please check the documentation in here:
https://github.com/mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/models/ngspice/testing/README.md

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@proppy and @mkkassem Changes in the documentation has been finalized.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

https://github.com/mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/models/xyce/testing/README.md

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@mkkassem @atorkmabrains I think you can easily create a separate pull request for the models changes this way:

# clone upstream repo
git clone https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr
cd globalfoundries-pdk-libs-gf180mcu_fd_pr
# add your fork remote
git remote add mabrains [email protected]:mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr.git
git fetch mabrains
# create feature branch
git checkout -b improve-models-docs
# checkout (only) models changes
git checkout mabrains/main models
# create a new collapsed commit for (only) models changes 
git commit -m 'models: improve documentation'
# push back to your fork
git push mabrains improve-models-docs
# create the pull request

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@proppy I have discussed that with @mithro before. That can't be done. Primitive renaming is a lateral change that affects everything and if you take one part and leave the other parts of the PDK unchanged, it will break the PDK. And there is another issue, doing PR will not work as well as the amount of changes are massive. And github doesn't allow that. I have shown this to @mithro and @mkkassem. That's why I was asking you guys to get the primitives renaming PRs in first before we change anything.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@atorkmabrains I'm confused, I don't think the README changes to the models/ directory that we're discussing here are related to the cell renaming changes? So they should be PR-able and mergeable separatly.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@proppy The documentation will not be aligned with what you have in this repo. Even if we only take readme changes, if you use it as is it will be broken.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@proppy Also, looking at the commands above, this will take everything in models folder including the renaming part and most likely github PR will not allow it.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

Even if we only take readme changes, if you use it as is it will be broken.

The renaming changes don't seems to affect the models, ex: https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/pull/48/files, so if you were checking-in the models change that include rename + docs in this PR, it wouldn't be broken, it would just be inconsistent with the rest of the PR until the other PR gets merged (simarly if you merge the standalone rename PRs first, it would also be inconsistent if you don't update the models at the same time).

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

What's important for those change is that we're able to run the tests for a given PR, so you need to merge the test either before (or with) the rename.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

Basically if you:

1. take the instruction in #69 (comment)
2. force push to #33 branches (https://github.com/mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr/tree/primitives_names_part2)
3. mark it as fixing this issue

That should be enough to provide a coherent set of change to review and merge (that include both naming change and appropriate documentation update to evaluate it).

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@proppy the renaming of the primitives is split over 18 PRs due to it's sheer amount of changes. The PR that you are referring to is just one out of 18. Please check with @mithro I have discussed this with him about 4 months ago.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@atorkmabrains, yes but that particular PR (#33) should be revieweable/mergeable independently of the remaining 17 as long we integrate it with the rest of the models change as shown in #69 (comment) and #69 (comment).

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@FaragElsayed2 Please take a look here as well.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@atorkmabrains I have updated all docs related to models and it is merged in efabless version.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.

@FaragElsayed2 Let's keep this issue open until it's added to Google here.

from globalfoundries-pdk-libs-gf180mcu_fd_pr.