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

Refactor value generators in the simulator #1060

Merged
merged 17 commits into from
Jul 20, 2023
Merged

Refactor value generators in the simulator #1060

merged 17 commits into from
Jul 20, 2023

Conversation

konnov
Copy link
Contributor

@konnov konnov commented Jul 19, 2023

Closes #954. To support generation of maps over infinite co-domains, I had to refactor the interface of pick in RuntimeValue. I like the new interface better, as it clearly exposes an iterator instead of doing tricky arithmetic over indices.

This is a breaking change. Since it changes how oneOf() picks values, the seeds in the tests are not valid anymore.

I have an impression that the simulator is slower now. Have to do some measurements.

  • Tests added for any new code
  • Entries added to the respective CHANGELOG.md for any new functionality

@konnov konnov requested review from thpani and shonfeder July 19, 2023 15:09
) as BS from "./BinSearch"
export BS.*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thpani did I just break the apalache integration?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, although I don't understand why it breaks with the export 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's maybe disable this test for now, as it waiting for the fix similar to #1051

Copy link
Contributor

@thpani thpani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about oneOf(Nat), otw LGTM

CHANGELOG.md Outdated Show resolved Hide resolved
quint/cli-tests.md Show resolved Hide resolved
// An infinite set, pick an integer from the range [-2^255, 2^255).
// TODO: make it a configurable parameter:
// https://github.com/informalsystems/quint/issues/279
.or(just(-(2n ** 255n) + this.rand(2n ** 256n)))
Copy link
Contributor

@thpani thpani Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the infinte set be Nat and shouldn't the lower bound then be non-negative?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, the implementation of pick for Int/Nat checks that and uses the absolute value in case of Nat. I just do not want to differentiate them here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Maybe leave a comment then, so it's not confusing in the future?

@konnov konnov enabled auto-merge (squash) July 20, 2023 11:27
@konnov konnov merged commit 71726a5 into main Jul 20, 2023
15 checks passed
@konnov konnov deleted the igor/maps954 branch July 20, 2023 13:59
@shonfeder shonfeder mentioned this pull request Aug 3, 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

Successfully merging this pull request may close these issues.

REPL returns <undefined value> instead of failing, when picking from maps with unbounded (Int) codomains
2 participants