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.

Sign up for GitHub

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

Jump to bottom

Hypothesis for parameters #6

Merged
merged 31 commits into from
Dec 14, 2023
Merged

Hypothesis for parameters #6

merged 31 commits into from
Dec 14, 2023

Conversation

AdrianSosic
Copy link
Collaborator

This PR introduces hypothesis strategies for the parameters package and rewrites the parameter testing logic:

  • The previous hand-selected set of serialization tests is replaced with hypothesis-generated test scenarios, covering a much broader range.
  • Bugs encountered during this process were fixed and the parameter validation logic was correspondingly extended.
  • A new set of "validation" tests is now responsible for the scenarios that are not covered by hypothesis (i.e. exception tests for invalid parameter specifications). These replace the previous incomplete set of parameter tests and now thoroughly cover all parameter validation steps by triggering the expected errors in a systematic way.

@AdrianSosic AdrianSosic self-assigned this Nov 28, 2023
AVHopp
AVHopp reviewed Nov 29, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@Scienfitz Scienfitz added enhancement Expand / change existing functionality new feature New functionality labels Dec 4, 2023
@AdrianSosic AdrianSosic force-pushed the hypothesis/parameters branch from ef80dc6 to ae26d50 Compare December 7, 2023 13:22
@AdrianSosic AdrianSosic mentioned this pull request Dec 8, 2023
Merged
@AdrianSosic AdrianSosic added tests Related to testing. and removed new feature New functionality labels Dec 8, 2023
@AdrianSosic AdrianSosic force-pushed the hypothesis/parameters branch from ae26d50 to 5056111 Compare December 8, 2023 14:37
@AdrianSosic AdrianSosic removed the enhancement Expand / change existing functionality label Dec 8, 2023
@AdrianSosic AdrianSosic mentioned this pull request Dec 8, 2023
Merged
Scienfitz
Scienfitz reviewed Dec 8, 2023
View reviewed changes
baybe/parameters/custom.py Show resolved Hide resolved
baybe/parameters/custom.py Show resolved Hide resolved
tests/hypothesis_strategies.py Show resolved Hide resolved
tests/hypothesis_strategies.py Show resolved Hide resolved
tests/hypothesis_strategies.py Outdated Show resolved Hide resolved
tests/hypothesis_strategies.py Show resolved Hide resolved
tests/hypothesis_strategies.py Outdated Show resolved Hide resolved
tests/hypothesis_strategies.py Outdated Show resolved Hide resolved
tests/serialization/test_parameter_serialization.py Show resolved Hide resolved
baybe/parameters/numerical.py Show resolved Hide resolved
@Scienfitz Scienfitz added this to the Hypothesis milestone Dec 8, 2023
AVHopp
AVHopp approved these changes Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

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

All of my comments are rather minor, so approve :)

baybe/exceptions.py Outdated Show resolved Hide resolved
baybe/parameters/categorical.py Show resolved Hide resolved
baybe/parameters/substance.py Show resolved Hide resolved
tests/hypothesis_strategies.py Show resolved Hide resolved
tests/hypothesis_strategies.py Outdated Show resolved Hide resolved
tests/hypothesis_strategies.py Outdated Show resolved Hide resolved
@AdrianSosic
Fix strategy for active_values
69c4140 
* use only randomness from built-in strategies: HypothesisWorks/hypothesis#3810
* the lower bound should be 1
* the now used hypothesis strategy further allows for shrinkage
@AdrianSosic
Add missing colons to class keyword
08cc332
@AdrianSosic
Make decorrelation strategy public
977363f
@AdrianSosic
Simplify exception docstring
8630fa8
@AdrianSosic
Refactor substance parameter validator
4cded72 
* Exceptions are now collected into an ExceptionGroup
* Added an additional check to identify duplicate substances
@AdrianSosic
Replace random.randint with hypothesis strategy
5d0f85b
@AdrianSosic AdrianSosic force-pushed the hypothesis/parameters branch from 774e228 to 5d0f85b Compare December 13, 2023 07:31
@AdrianSosic AdrianSosic merged commit e131354 into main Dec 14, 2023
8 checks passed
@AdrianSosic AdrianSosic deleted the hypothesis/parameters branch December 14, 2023 12:26
AdrianSosic added a commit that referenced this pull request Dec 19, 2023
@AdrianSosic
Hypothesis for targets and intervals (#56)
c21197a 
This PR brings hypothesis testing for targets and intervals and follows
the same approach as described in #6.

To do so, it also enables proper deserialization of target subclasses
via the base class using the same "tagged union approach" as for the
remaining BayBE components. Since providing type specification for
targets was not enforced previously, a temporary backward-compatible
workaround is added that fills missing type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AVHopp AVHopp AVHopp approved these changes

@Scienfitz Scienfitz Scienfitz approved these changes

Assignees

@AdrianSosic AdrianSosic

Labels
tests Related to testing.
Projects
None yet
Milestone
Hypothesis
Development

Successfully merging this pull request may close these issues.
3 participants
@AdrianSosic @Scienfitz @AVHopp