Simplify QC generators for commonly-used types. #2768
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rvl
force-pushed
the
jonathanknowles/2695/weeder
branch
from
48c39be
to
4eeb09d
Compare
jonathanknowles
force-pushed
the
jonathanknowles/2695/weeder
branch
2 times, most recently
from
b37f8bd
to
c2fad8d
Compare
jonathanknowles
changed the title
Spotted this refactoring opportunity while reviewing test coverage for UTxO properties. The following are equivalent: - Set.null (a `Set.intersection` b) - a `Set.disjoint` b
This commit:
- Renames `genCoinAny` to `genCoinFullRange`, making it clear from the
name that values are chosen from across the full range of a `Coin`.
- Gives the generator a slight bias toward the limits of the range,
but otherwise preserves the uniform distribution across the whole
range.
Instead of having `genCoinSmall` and `genCoinLarge` variants, we instead define a single shared generator that makes use of the implicit `size` parameter. This commit also adjusts the call sites of these generators to use one of the following generators, depending on the use case: - `genCoin` - `genCoinPositive` - `genCoinFullRange`
jonathanknowles
force-pushed
the
jonathanknowles/2695/weeder
branch
from
c2fad8d
to
ff2f2bc
Compare
In this test, the expected numbers of inputs are highly sensitive to the size distribution of token bundles within generated transaction outputs. This commit imposes a limit on the sizes of generated token bundles, so that the expected numbers of inputs are as close as possible to the previous set of values.
With large values, we want to limit the number of results returned in order to avoid processing long lists of shrunken values.
| estimateMaxInputsTests @IcarusKey | ||
| [(1,73),(5,67),(10,63),(20,52),(50,14)] | ||
| [(1,73),(5,69),(10,64),(20,54),(50,17)] |
There was a problem hiding this comment.
I did try to retain the old values here, but in the end settled for making the (1, *) pairs match, and getting the rest to match as closely as possible.
|
bors try |
tryBuild failed: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
bors try |
tryBuild succeeded: |
rvl
commented
Jul 27, 2021
|
@rvl wrote:
That's next on my list: will do this when I get a spare moment! Thanks for the review. 👍🏻 |
iohk-bors bot
added a commit
that referenced
this pull request
Aug 17, 2021
2829: Rewrite generators to use the QC size parameter r=jonathanknowles a=jonathanknowles ### Issue Number Follow-up from #2768 Supports work for #2819 ### Comments This PR adjusts generators for the following types to use the QC size parameter: - `Address` - `Hash "Tx"` - `TxIndex` (`Word32`) - `TxIn` - `TxOut` - `UTxO` - `UTxOIndex` Some minor adjustments to coverage conditions were necessary (but surprisingly few). This PR also: - Adds the module `UTxO.Gen`, so that we can generate values of `UTxO` without having to generate indices (which add extra overhead), and redefines the `UTxOIndex` generators in terms of these generators. - Adds the functions `genSized2` and `genSized2With`, which restore size linearity to generators of compound values (those defined in terms of other generators). Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 17, 2021
2829: Rewrite generators to use the QC size parameter r=jonathanknowles a=jonathanknowles ### Issue Number Follow-up from #2768 Supports work for #2819 ### Comments This PR adjusts generators for the following types to use the QC size parameter: - `Address` - `Hash "Tx"` - `TxIndex` (`Word32`) - `TxIn` - `TxOut` - `UTxO` - `UTxOIndex` Some minor adjustments to coverage conditions were necessary (but surprisingly few). This PR also: - Adds the module `UTxO.Gen`, so that we can generate values of `UTxO` without having to generate indices (which add extra overhead), and redefines the `UTxOIndex` generators in terms of these generators. - Adds the functions `genSized2` and `genSized2With`, which restore size linearity to generators of compound values (those defined in terms of other generators). Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 17, 2021
2829: Rewrite generators to use the QC size parameter r=jonathanknowles a=jonathanknowles ### Issue Number Follow-up from #2768 Supports work for #2819 ### Comments This PR adjusts generators for the following types to use the QC size parameter: - `Address` - `Hash "Tx"` - `TxIndex` (`Word32`) - `TxIn` - `TxOut` - `UTxO` - `UTxOIndex` Some minor adjustments to coverage conditions were necessary (but surprisingly few). This PR also: - Adds the module `UTxO.Gen`, so that we can generate values of `UTxO` without having to generate indices (which add extra overhead), and redefines the `UTxOIndex` generators in terms of these generators. - Adds the functions `genSized2` and `genSized2With`, which restore size linearity to generators of compound values (those defined in terms of other generators). Co-authored-by: Jonathan Knowles <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
#2695
Overview
This PR simplifies the generators (and shrinkers) for the following types:
CoinTokenQuantityTokenPolicyIdTokenNameAssetIdEach type now has a single generator and shrinker that relies on the QC size parameter. These have the following form:
genTypeshrinkTypeWhere strictly-positive values are required, we also have:
genTypePositiveshrinkTypePositiveWhere values across the full range of a type are required, we also have:
genTypeFullRangeshrinkTypeFullRangeThis PR also adjusts coverage checks for some properties.
Future work
In a future PR, we can simplify generators for
TokenMap,TokenBundle,UTxOIndex,TxIn,TxOut.