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

unify drc checks semantic #54

Open
proppy opened this issue Mar 30, 2023 · 10 comments
Open

unify drc checks semantic #54

proppy opened this issue Mar 30, 2023 · 10 comments

Comments

@proppy
Copy link
Member

proppy commented Mar 30, 2023

There are currently 2 differents way used to test DRC assertions in the recent PRs:

  • DRC check functions like .width that evaluate a condition and returns a list of violating shape: the condition is expressed as the passing condition before .output is called on the resulting shapes.
  • DRC selector functions like .with_length that evaluate a condition and returns a list of matching shapes: the condition is inverted to express the failing condition before .output is called on the resulting shapes.

We should make sure we use matching semantic to ease readability and maintainance by either:

  • (a) using inverse function like .without_length so that the we always express passing condition.
  • (b) using the universal drc method that always encode passing condition using an expression syntax.

I think I'd personally favor (b) but maybe @klayoutmatthias or @mithro have strong opinions on this?

@proppy proppy changed the title unifed drc check semantic unify drc check semantic Mar 30, 2023
@atorkmabrains
Copy link
Collaborator

atorkmabrains commented Mar 30, 2023

@proppy drc command is very slow and it's not recommended for performance concerns.

@proppy
Copy link
Member Author

proppy commented Mar 30, 2023

@atorkmabrains interesting, the documentation here https://www.klayout.de/doc/about/drc_ref_layer.html#drc mention

improved performance in some applications

does that mean that our drc deck has applications are not covered by those improvement?

/fyi @klayoutmatthias @mithro

@atorkmabrains
Copy link
Collaborator

@proppy That was the recommendations from @klayoutmatthias to avoid the use of drc command.

@klayoutmatthias
Copy link

The drc function is not slow in general, but in this particular case it lacks an optimization (forced merging of inputs), which makes it slower. This is addressed by KLayout/klayout#1195, but I did not have time yet to look into that problem. As of now, it is better to use the plain DRC functions ("space", "width", ...) when possible.

Matthias

@proppy
Copy link
Member Author

proppy commented Mar 31, 2023

@klayoutmatthias Thanks for chipping in!

@atorkmabrains I'd be in favor of keeping this open until there is progress and KLayout/klayout#1195 and then consider switching to drc.

@atorkmabrains
Copy link
Collaborator

BTW @proppy, @klayoutmatthias reviewed this rule deck and he was the one who suggested the removal of drc command.

Thanks @klayoutmatthias on your help back then. It would be nice to get your feedback again this time. BTW, we have integrated everything in efabless version.

@klayoutmatthias
Copy link

@atorkmabrains Of course - you mean you want feedback on the current version?

@atorkmabrains
Copy link
Collaborator

@klayoutmatthias yes, please. Please check efabless version.

@atorkmabrains
Copy link
Collaborator

@atorkmabrains
Copy link
Collaborator

@klayoutmatthias BTW, this time we have our DRC regression as CI. This way we could detect if the changes might have impact on the correctness of output.

@proppy proppy changed the title unify drc check semantic unify drc checks semantic Apr 4, 2023
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

No branches or pull requests

3 participants