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

Make clobber interact appropriately with default values #1146

Closed
nmoroze opened this issue Sep 19, 2022 · 0 comments · Fixed by #1222
Closed

Make clobber interact appropriately with default values #1146

nmoroze opened this issue Sep 19, 2022 · 0 comments · Fixed by #1222

Comments

@nmoroze
Copy link
Contributor

nmoroze commented Sep 19, 2022

I think the clobber flag to chip.set() currently works in an unintuitive way. The problem is it will always overwrite an "empty" value, even if it has been explicitly set. E.g.:

>>> import siliconcompiler
>>> chip = siliconcompiler.Chip('test')
>>> chip.set('option', 'remote', False)
>>> chip.get('option', 'remote')
False
>>> chip.set('option', 'remote', True, clobber=False)
>>> chip.get('option', 'remote')
True

I think it should not be considered correct behavior for the second chip.set() to override the value that the user has explicitly set. This behavior caused the server tests to fail in the first iteration of #1145, and I think a proper solution would be to fix this issue.

I think the fix for this would be to no longer set the 'value' field of parameters when the schema is first initialized, and rather just have chip.get() fall back to the defvalue transparently when no value is set. Then, sets with clobber=False will only take effect if the value field does not exist.

I'd like to refactor _search() at some point soon, after I finish splitting out schema access into a separate class. Once that happens, I can make this change and fix this issue.

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 a pull request may close this issue.

1 participant