LVS/DRC tests never seems to fails

[proppy]

Expected Behavior

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/rules/klayout/drc/testing/run_regression.py and https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/rules/klayout/lvs/testing/run_regression.py should populate an error status that represents the aggregated failure and success state of the test cases.

Actual Behavior

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/rules/klayout/drc/testing/run_regression.py and https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/rules/klayout/lvs/testing/run_regression.py never seems to exit with a status != 0.

[proppy]

ideally we would have a synthetic PR that demonstrate that the current regression CI setup fails when an pcell/gds is submitted.

[atorkmabrains]

@proppy I'm not sure what you want to do here. Make a PR that would fail CI?

[proppy]

@atorkmabrains yes, I'm trying to assess if the python scripts in https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/main/rules/klayout/drc/testing/ can ever exit with a status != 0. From reviewing the different code paths that doesn't seems to be the case.

/cc @QuantamHD

[mithro]

Yes please, send a pull request with a DRC error in it to demonstrate the automation would detect the problem.

[atorkmabrains]

@proppy and @mithro we had discussed this more than 3 months ago and we had a pull request to make sure that is the case that was not integrated in the repo:

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/pull/51

[atorkmabrains]

@mithro and @proppy Could you please integrate all the pending PRs before evaluation?

[atorkmabrains]

@mithro the main branch here still uses the original device names. We have stopped working on fixes until you approve those in. I urge you to give me maintainer access to straighten the PDK out for proper usage??

[proppy]

@atorkmabrains commented in #51

[mkkassem]

@QuantamHD @proppy should we close this one and point to #51 ?

[proppy]

@QuantamHD @proppy should we close this one and point to https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/pull/51 ?

We could update #51 description to mention that it "Closes #64" (see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)

[proppy]

@mkkassem @atorkmabrains I think you can easily create a separate pull request for the drc 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 git@github.com:mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr.git
git fetch mabrains
# create feature branch
git checkout -b improve-drc-failure
# checkout (only) drc changes
git checkout mabrains/main rules/klayout/drc
# create a new collapsed commit for (only) drc changes 
git commit -m 'rules/klayout/drc: improve drc failure mode and documentation'
# push back to your fork
git push mabrains improve-drc-failure
# create the pull request

and do something similar that capture the LVS changes

# 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 git@github.com:mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr.git
git fetch mabrains
# create feature branch
git checkout -b improve-lvs-failure
# checkout (only) lvs changes
git checkout mabrains/main rules/klayout/lvs
# create a new collapsed commit for (only) lvs changes 
git commit -m 'rules/klayout/lvs: improve lvs failure mode and documentation'
# push back to your fork
git push mabrains improve-lvs-failure
# create the pull request

[atorkmabrains]

@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.

[proppy]

For DRC: The renaming changes don't seems to affect the drc checks/rules (I couldn't find any drc renaming changes in https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/pulls.

So you should be able to following the first set of instruction in https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/issues/64#issuecomment-1311154847 to submit a PR that only include the DRC error checking and documentation changes.

[proppy]

For LVS, the renaming changes do affect the testcases, so I think the approach we want to follow here is a slighty revised version of what I suggested earlier:

# 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 git@github.com:mabrains/globalfoundries-pdk-libs-gf180mcu_fd_pr.git
git fetch mabrains
# create feature branch
git checkout -b improve-lvs-failure
# checkout (only) change that improve lvs failure mode and their documentation
git checkout mabrains/main rules/klayout/lvs/*.{md,py,lvs} rules/klayout/lvs/testing/{Makefile,*.py}
# create a new collapsed commit for (only) lvs changes 
git commit -m 'rules/klayout/lvs: improve lvs failure mode and documentation'
# push back to your fork
git push mabrains improve-lvs-failure
# create the pull request

And then we should be able to rebase the opened renaming PRs on top of this change, directly from the github UI (as they wouldn't be conflicting) and be able to exercise the test suite to review them.

[FaragElsayed2]

@atorkmabrains This issue has been addressed in Efabless/PV repo

[atorkmabrains]

@proppy This has been addressed in PV repo. Also, we moved LVS to PV repo and no longer part of PR repo.

[atorkmabrains]

I'll close the ticket.