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

Introduce ModelVarContext as a generalisation of ModelLookup and ModelFindVariables #25

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

jorisdral
Copy link
Collaborator

@jorisdral jorisdral commented Oct 14, 2024

Occurrences of ModelFindVariables and ModelLookup in the InLockstep class are replaced by the newly exposed ModelVarContext. A ModelFindVariables can be recovered from a ModelVarContext using the new findVars functions. A ModelLookup can be recovered from a ModelVarContext using the new lookupVars function. Since these functions can be recovered from ModelVarContext, existing tests are guaranteed to be adaptable to the new InLockstep API.

Motivation: previously in the InLockstep class, member functions would be passed a ModelFindVariables or a ModelLookup, but never both. In practice this turned out to be too restrictive, because one might want access to both in the same function, see IntersectMBO/lsm-tree#431. For example, arbitraryWithVars was previously only passed a ModelFindVariables, but without a ModelLookup one can not filter these variables based on the (modelled) outcome of the corresponding actions. The use case for #431 in particular was to filter variables that reference stateful handles (e.g., file handles) for handles that are open. Because of this filtering, one can skew the action distribution to generate more actions on open handles. It is often more interesting to test actions on open handles than it is to test actions on closed handles, which will presumably always return the same error.

@jorisdral jorisdral self-assigned this Oct 14, 2024
@jorisdral jorisdral force-pushed the jdral/model-var-context branch from 8a724c0 to 71616aa Compare November 4, 2024 10:27
@jorisdral jorisdral marked this pull request as ready for review November 4, 2024 10:28
@jorisdral jorisdral requested a review from dcoutts November 4, 2024 10:29
@jorisdral jorisdral force-pushed the jdral/model-var-context branch from 71616aa to 1852cf8 Compare November 4, 2024 10:32
Copy link
Member

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

LGTM, but I'll see what @edsko thinks! :-)

CHANGELOG.md Outdated Show resolved Hide resolved
Occurrences of `ModelFindVariables` and `ModelLookup` in the `InLockstep` class
are replaced by the newly exposed `ModelVarContext`. A `ModelFindVariables` can
be recovered from a `ModelVarContext` using the new `findVars` functions. A
`ModelLookup` can be recovered from a `ModelVarContext` using the new
`lookupVars` function. Since these functions can be recovered from
`ModelVarContext`, existing tests are guaranteed to be adaptable to the new
`InLockstep` API.

Motivation: previously in the `InLockstep` class, member functions would be
passed a `ModelFindVariables` or a `ModelLookup`, but never both. In practice
this turned out to be too restrictive, because one might want access to both in
the same function, see IntersectMBO/lsm-tree#431. For
example, `arbitraryWithVars` was previously only passed a `ModelFindVariables`,
but without a `ModelLookup` one can not filter these variables based on the
(modelled) outcome of the corresponding actions. The use case for #431 in
particular was to filter variables that reference stateful handles (e.g., file
handles) for handles that are open. Because of this filtering, one can skew the
action distribution to generate more actions on open handles. It is often more
interesting to test actions on open handles than it is to test actions on closed
handles, which will presumably always return the same error.
@jorisdral jorisdral force-pushed the jdral/model-var-context branch from 1852cf8 to a59f0e6 Compare December 3, 2024 16:01
@jorisdral jorisdral merged commit 5170357 into main Dec 3, 2024
7 checks passed
@jorisdral jorisdral deleted the jdral/model-var-context branch December 3, 2024 16:05
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.

2 participants