-
Notifications
You must be signed in to change notification settings - Fork 35
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
use dict
to declare type
#117
Comments
For later reference: this time the above error was caused by dumping to |
I think that your proposed action item is a nice long-term solution insofar as we hope to introduce more types. However, the difficulty that you have encountered---of failing to use a
Note that these actions are similar to the values of the |
I agree that simplicity is generally to be preferred. However, I already use an extra
Having said the above, I admit that for human input, initializing a symbol table as a The use of a single |
Before beginning, note that the proposed action items in the OP involve changes that break a lot of existing code or otherwise require wrappers for backwards compatibility. A general strategy I recommend is
Addressing your specific comment, let us be concrete. I am referring specifically to variable representations in the classes
I am strongly opposed to the first item. There is much code that relies on there being You condemn "the implicit convention" broadly, but can you give specific motivation for this particular issue (referring back to the OP)? Finally, I want to emphasize that introducing symbol tables into the |
I just removed the |
Can someone give an example of how user code would be modified under the proposed change? A before/after example would be good, perhaps one of the standard examples that we use? |
I think that the OP is well argued and that the action items should be performed for version 1.3.0. I recommend that this change is not made for version 1.2.0 for the same reasons that motivate a period of feature-freeze (i.e., no additional features; only bug-fixes) before release. After the OP, discussion turned to other motivations and possible uses of symbol tables. While I think these could be valuable, I want to keep this issue focused on the action items of the OP. After such a @murrayrm: I skimmed the examples but did not find one that would be convenient for demonstration. While there are examples involving variables of all three types, the user-facing code does not reveal them. E.g., the For discussion, here is a small example of how we currently may create a specification involving all three types. Note that a common alternative style is to create the from tulip.spec import GRSpec
f = GRSpec()
f.sys_vars = {'v1': 'boolean', 'v2': (0,3), 'v3': ['tofu', 'beef', -1, 2]}
f.sys_init = ['v1', 'v2 = 0', 'v3 = "beef"']
f.sys_safety = ['v1 -> X(v3 = "tofu")'] Following the comment by @johnyf above, we may change the line above in which f.add_var('v1')
f.add_var('v2', (0,3))
f.add_var('v3', 'str', ['tofu', 'beef', -1, 2]) where f.add_var('v2', (0,3), owner='env') |
After implementing these approaches in the package My proposed style to define a full symbol table is cumbersome for external users. The availability of a It also works for multiplayer games, whereas the env-sys classification would need to be modified (i.e., tracking an "owner" is necessary for representing games with multiple players, irrespective of whether detailed type information is provided). To avoid confusion below: there are roughly 4 possible symbol tables:
The formats 2, 3, 4 are documented in As commented here, there is also the convenience function I think that each interface has its purpose, because use cases at different levels of involvement (by the same person) with the backend admit different input as more convenient. I have kept all of 2, 3, 4, though I use mostly 3. It seems that more detailed input by humans is better accommodated by using a specification language (Promela, etc.) as input. Whether that should be typed or not is a different discussion. Footnote: |
I think that the action item to close this issue should be to add a method A method abstracts away from the internal implementation, allowing for flexibility. it does not require that the user know and manipulate the data structure directly (attributes of A more recent experiment of mine is A method as interface has the added advantage that it can execute code with arguments (as opposed to an attribute---even if it is The existing API of directly changing attributes will remain unaffected. In the future, if a good reason is found, it could be modified. My expectation is that all input will shift to a suitable input language with proper variable declarations, thus offering an earlier entry point to users. |
From a user perspective, I like something like this:
much better than this
Of course, these are not incompatible (the |
I agree, in b = dd.bdd.BDD()
b.add_var('a')
b.add_var('b', level=5) By default, new bits are added at the next level that is not occupied. Passing a level explicitly overrides this behavior. The method My main motivation when implementing |
More arguments in favor of not manually defining per variable owners, to add to the arguments above:
Exotic semantics (saturating "integer variables", modwrap) suggest types. They are implicit and confusing. I never use them. I removed the silent automated generation of type constraints from Of course the tools can be annotating variables in symbol tables however is needed by the code. But the user interface need not reflect this activity. |
@johnyf any updates on this? I am available to review any work you may have on this. Alternatively, I can try to implement the proposed |
I added a method Boolean-valued variables are given as positional arguments (the absence of a type hint signifies that a variable is Boolean-valued). Other variables are given as keyword arguments, where the value is either a pair of integers, or a sequence of strings. For example: f.declare(
'v1',
v2=(0, 3),
v3=['tofu', 'beef', '-1', '2']) Passing f.declare('v4', env=True) Giving type hints as keyword values, instead of separate arguments is necessary if
Reserving the keyword "env" precludes defining a variable named "env" via a call to Redeclarations are allowed, but must match existing type hints and owners, thus have no effect (the docstring of method f.declare('v1')
f.declare(v2=(0, 3))
f.declare(v3=['tofu', 'beef', '-1', '2']) Regarding earlier comments from aboveUntil now I have experimented with various approaches in
This approach is flexible and not repetitive. Remarks from usage experience:
|
PEP 20: explicit is better than implicit. Inferring the type of a variable from the data type used to describe its domain is fragile. As of c8f8217:
boolean
signifies that the variable is of Boolean typetuple
signifies that the variable is of integer typelist
signifies that the variable is of enumerated type, where its possible values are restricted to strings (syntactically defined enclosing them in double quotes).This approach mixes the type definition with the data type that one will use to represent the domain itself.
The most common error that I keep repeating is to pass an iterable of length 2 to define the limits of an integer domain, but not use a
tuple
. For example, if this is happening outsidetulip
and attention is focused on some other detail, then it is quite easy to not use atuple
, or perhaps use aset
instead of alist
.The use of types above is to be avoided, for the same reasons that
isinstance
should be avoided, when possible.The action item is to use two separate fields in the
dict
of a variable:"type"
key that takesstr
values, in particular in{"bool", "int", "str"}
It may be tempting to admit multiple alternatives for each type, e.g., both "boolean" and "bool". By PEP 20: "There should be one-- and preferably only one --obvious way to do it." so this temptation is resisted here. The above type names have been borrowed from the implementation language (Python).
"dom"
key that takes values:bool
the value is ignoredint
the value is an iterable of length 2 that containsint
str
the value is an iterable ofstr
Note that this issue does not intend to suggest that a fix is necessary, perhaps rewriting from scratch some parts based on the experience gained so far will yield much better results.
The text was updated successfully, but these errors were encountered: