Fix dtypes related to floating point precision #254
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
AdrianSosic
reviewed
May 29, 2024
There was a problem hiding this comment.
Hi @Scienfitz, thanks for the PR. Here a few comments to discuss
AdrianSosic
changed the title
Scienfitz
force-pushed
the
fix/dtypes
branch
2 times, most recently
from
7946692 to
b735ef2
Compare
AdrianSosic
reviewed
May 29, 2024
AdrianSosic
reviewed
Jun 4, 2024
baybe/simulation/lookup.py
Outdated
| # could also be implemented for approximate matches | ||
| elif isinstance(lookup, pd.DataFrame): | ||
| # Enforce correct float precision in the lookup dataframe | ||
| float_cols = lookup.select_dtypes(include=["float"]).columns | ||
| lookup[float_cols] = lookup[float_cols].astype(DTypeFloatNumpy) |
There was a problem hiding this comment.
I think this is problematic since you mutate the incoming object, i.e. this will also affect the object outside the scope of this function
There was a problem hiding this comment.
ok changed it and also moved to simulate_experiment where it creates a copy before modifying, this should also do the modifications fewer times which possibly affects speed
AdrianSosic
approved these changes
Jun 4, 2024
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.
Since floating point precision can be controlled via env vars (#226) various problems have surfaced letting tests fail in single precision.
This PR fixes those. They were mostly related to the way
valuesandcomp_dfwere created for parameters,selectionwas treated inSubSelectionConditionand alookupin a different float precision being used in a simulation.The only remaining issues with test in single precision are numerical instabilities (out of scope)