-
Notifications
You must be signed in to change notification settings - Fork 122
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
Systematic inclusion of integers in univ and unclear default bound #199
Comments
I can look into this, but probably only once the Spring 2023 semester is over. Note that the example above only yields an instance if "prevent overflows" is disabled. So perhaps we're reasoning about, not
which produces instances with 1
we get instances with both 1 and 5 |
Indeed it seems that the example is behaving correctly according to the wrap-around semantics of integers (without the prevent overflows). 3 is actually -1 with the bitwidth of 2 and if we ask in the evaluator for the value of However, I think there is an issue when we explicitly state that we want a scope of 0 for This is related to the first and second points: we could default default the scope of I think the third point is a bug, and its not only related to integers. The textual representation is showing other stuff it should not, for example, information related to where the the loopback occurs in the trace or the size of the trace. Maybe we could move that point into a separate issue (maybe include it in issue #193). |
Now I noticed there was a related discussion in issue #30. |
Yeah, I'm a bit more extreme than you in #30 in that I would advocate systematically emitting a warning when arithmetic is used, unless an explicit bound on Int is set.
OK. |
About the 4th point (with the example) now I understand that it indeed behaves correctly according to the wrap-around semantics of integers, but as already proposed in #30, a warning could be raised because the Int bound is not large enough to hold all the integer constants explicitly present in the model. About the 3rd point, I agree with Alcino: let's put it in a separate issue because it is not only about integers. I created the new issue #207 because issue #193 is related to split panes, which I find quite different. |
To add to the confusion, if you set the scope of |
Picking this up after re-reading the above comments. I propose the following changes: (1) Alloy must issue a warning if an integer literal is used in the model that cannot be expressed in the current bitwidth. This is imperfect (we wouldn't catch, say,
Moreover, a (2) It seems vital that the evaluator reflects the model's semantics in its answers. This means it must reflect the chosen bitwidth for the current run, including evaluating cardinality to the empty set if there are no Int atoms. Comparison operators would continue to work as they do now, where the empty set is treated like Here I am a bit concerned about why Kodkod will "adjust" a 0 bitwidth as @nmacedo points out. I know there are situations where upper and lower bounds don't suffice to express numeric scopes, and so some cardinality constraints must be added. But last I checked, these were expressed using I want to look into this more on the Kodkod/Pardinus side before attempting the change. (3) I think we should more broadly discuss changing the default bitwidth, because it might impact many pre-existing models. I know some of mine, at least, assume the default. The proposed warning above will help catch many of these backward-compatibility issues, especially if we give a warning when cardinality is used at all, but it changes the issue from a silent failure to a noisy one. Personally, I'd also favor a (4) As I prototyped this some, I realized that there is some syntactic confusion in the evaluator, since |
Some aspects of the treatment of integers need clarification / improvement:
yields an instance. This is cool but I'm not sure this is explained somewhere.
The text was updated successfully, but these errors were encountered: