-
Notifications
You must be signed in to change notification settings - Fork 25
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
Expose MINBATCHTIMEOUT parameter in set_model interface #406
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #406 +/- ##
===========================================
+ Coverage 78.19% 78.38% +0.18%
===========================================
Files 97 97
Lines 7178 7204 +26
===========================================
+ Hits 5613 5647 +34
+ Misses 1565 1557 -8
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for turning that around so quick! Will definitely lead to easy performance gains.
Only one request from me
- Throw a warning if
MINBATCHSIZE>0
andMINBATCHTIMEOUT==0
to nudge the user. Something along the lines ofmin_batch_timeout was set with a non-zero min_batch_size. Performance may improve by passing in a small value (~10ms) for min_batch_timeout
- Throw an exception if
MINBATCHTIMEOUT>0 and (MINBATCHSIZE==0 or BATCHSIZE==0)
with the message thatmin_batch_size and batch_size must be non-zero if min_batch_timeout>0
Add those tests to ensure we're throwing those to test_errors.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really appreciate the new usages exceptions/warnings. Also nice work on adding the test to capture the warning
.github/workflows/run_tests.yml
Outdated
@@ -67,6 +67,13 @@ jobs: | |||
GCC_V: ${{ matrix.compiler }} # used when the compiler is gcc/gfortran | |||
|
|||
steps: | |||
# Free up some disk space | |||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooof really these are the kind of games that we have to play?Q
No description provided.