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

More explicit subsample bounds check #1545

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

SkBlaz
Copy link
Contributor

@SkBlaz SkBlaz commented Jul 23, 2022

The subsample float can be further validated via 0 <= subsample < 1 (not only the upper bound)

The subsample float can be further validated via 0<=subsample<1 (not only the upper bound)
@eddiebergman
Copy link
Contributor

Hi,

Thanks for the PR, I will take a look next week once I am back!

Sorry for the delay

Best,
Eddie

@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #1545 (8ab839d) into development (af9d469) will decrease coverage by 0.14%.
The diff coverage is 100.00%.

@@               Coverage Diff               @@
##           development    #1545      +/-   ##
===============================================
- Coverage        84.38%   84.23%   -0.15%     
===============================================
  Files              154      154              
  Lines            11694    11694              
  Branches          2038     2038              
===============================================
- Hits              9868     9851      -17     
- Misses            1282     1295      +13     
- Partials           544      548       +4     

Impacted file tree graph

@eddiebergman
Copy link
Contributor

So the test failures seem unrelated to this and I would be willing to just merge the changes.

@eddiebergman eddiebergman merged commit 7318999 into automl:development Aug 3, 2022
eddiebergman pushed a commit that referenced this pull request Aug 18, 2022
The subsample float can be further validated via 0<=subsample<1 (not only the upper bound)
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