Skip to content

Add is_close function for `Interval´ class#768

Open
AVHopp wants to merge 6 commits intomainfrom
bug/numerical_target_normalization
Open

Add is_close function for `Interval´ class#768
AVHopp wants to merge 6 commits intomainfrom
bug/numerical_target_normalization

Conversation

@AVHopp
Copy link
Copy Markdown
Collaborator

@AVHopp AVHopp commented Mar 26, 2026

This PR adds an is_close functionality for the Interval class to allow fuzzy comparisons of interval boundaries.

This fixed #767 by replacing the strict check for normalisation of an Interval against the Interval(0,1) by a fuzzy match against this interval. The functionality simply uses the np.isclose function to check both interval boundaries.

@AVHopp AVHopp self-assigned this Mar 26, 2026
Copilot AI review requested due to automatic review settings March 26, 2026 09:52
or (self.lower < number < self.upper)
)

def is_close(self, other: Interval, /) -> bool:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@Scienfitz @AdrianSosic do we want to add the possibility to pipe through **kwargs that are then piped through to np.isclose for finer control?

Copy link
Copy Markdown
Collaborator

@Scienfitz Scienfitz Mar 26, 2026

Choose a reason for hiding this comment

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

we dont need this function, we can rely on attrs builint funcitonality, you merely have to tell attrs please compare the "lower" and "upper" attributes using "np.isclose", then the entire IntervalA == IntervalB comparison will be fuzzy as we need
see my comment in #767

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Changed, please have a look: c81db9e

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Also note that I could not directly use np.isclose as this caused mypy to complain, I hence added a lambda as well as bool conversion in 343d7f8

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a fuzzy interval-boundary comparison utility to address overly strict normalization checks (Issue #767), replacing exact Interval(0, 1) comparisons with a tolerance-based approach.

Changes:

  • Added Interval.is_close() using numpy.isclose on both bounds.
  • Updated NumericalTarget.is_normalized to use Interval.is_close(Interval(0, 1)) instead of strict equality.
  • Added validation tests for closeness behavior and documented the change in the changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
baybe/utils/interval.py Introduces Interval.is_close() for fuzzy boundary comparison.
baybe/targets/numerical.py Switches normalization detection to use the new fuzzy comparison.
tests/validation/test_interval_validation.py Adds tests for Interval.is_close() behavior.
CHANGELOG.md Documents the new API and the normalization-check fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AVHopp AVHopp force-pushed the bug/numerical_target_normalization branch from 38e6696 to c81db9e Compare March 27, 2026 09:26
@Scienfitz Scienfitz added the bug Something isn't working label Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check for is_normalized for NumericalTarget is too strict

3 participants