Avoid splitting outputs in batches when nbatches == 1 #406
Conversation
This file contains hidden or 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
filipecosta90
previously approved these changes
Jun 8, 2020
Codecov Report
@@ Coverage Diff @@
## master #406 +/- ##
==========================================
+ Coverage 72.46% 72.60% +0.13%
==========================================
Files 21 21
Lines 4384 4424 +40
==========================================
+ Hits 3177 3212 +35
- Misses 1207 1212 +5
Continue to review full report at Codecov.
|
|
@filipecosta90 I don't understand the failing codecov/patch, what's the issue there? Probably it's the batch size check with TF and ONNX in 7ad0d18 |
I believe the reason it fails is due to patch coverage ( added code is 65% ) even tough on overall the project coverage will increase. IMO we can merge and address this in the future via unit testing ( same as other corner cases that are difficult to test on flows ) |
filipecosta90
approved these changes
Jun 10, 2020
filipecosta90
pushed a commit
that referenced
this pull request
Aug 24, 2020
* Avoid splitting outputs in batches when nbatches == 1 * Add batch size checks * Fix batch checks * Update readies * Add bad batching test
Merged
filipecosta90
added a commit
that referenced
this pull request
Sep 2, 2020
* Introduced readies submodule (#377) * Introduced readies submodule * Fix in paella * Updated get_deps.sh and docs * [WIP] add C API reference to docs using auto tool (#368) * [add] add C API reference to docs using auto tool ( python3 generate_llapi_reference.py ) * [add] generating C API reference on the fly * [add] moving api_reference under Developer Nav Co-authored-by: Luca Antiga <[email protected]> * [WIP] Enable AI.SCRIPTRUN on AI.DAGRUN* (#383) * [add] decoupled scriptrun command parsing from runtime scriptrun. * [add] split positive/negative tests on pytorch scriptrun. * [add] refactor AI.DAGRUN_RO and AI.DAGRUN to use the same code base (with read/write modes) * [add] added positive and negative tests for dagrun with scriptrun * [add] updated documentation to reflect scriptrun support on dagrun * [add] added example enqueuing multiple SCRIPTRUN and MODELRUN commands within a DAG * Add support for variadic arguments to SCRIPT (#395) * Add support for variadic arguments to SCRIPT * Add negative errors * atomic ref count (#403) * Multi-platform build (#398) * CircleCI: increased GPU test timeout to 40m (#404) * CI: platform-build-defs -> on-master-and-version-tags (#405) * Llapi updates (#400) * added low level api return redis types * added variadic to llapi * fixed memory issue for params array re-alloc * Avoid splitting outputs in batches when nbatches == 1 (#406) * Avoid splitting outputs in batches when nbatches == 1 * Add batch size checks * Fix batch checks * Update readies * Add bad batching test * Update README.md * CircleCI: fixed redis installation in coverage builds (#423) * Docker6.0.5 (#427) * Update Dockerfile * Update Dockerfile.arm * Update Dockerfile.gpu * submodule pull moved up (#428) * Update mkdocs.yml (#432) * [add] Add relevant RedisAI config entries to the INFO output, so that standard monitoring systems would be able to monitor them. (#396) * Updated support email and Orobix to Tensorwork on ramp file (#436) * [add] updated support email and Orobix to Tensorwork on ramp file Co-authored-by: Sherin Thomas <[email protected]> * CircleCI: Fixed problem with GPU testing (#440) * Fixed platforms build problem (#441) * Fixed platforms build problem * Build: updated Redis versions * Build: another Redis version update * Dependencies in ramp.yml (#444) * Fixed flagged as "getkeys-api" during the registration ( AI.DAGRUN, AI.DAGRUN_RO, AI.MODELRUN, AI.SCRIPTRUN ) (#438) * [wip] wip on fixing the commands being flagged as getkeys-api during the registration. ( AI.MODELRUN, AI.SCRIPTRUN, AI.DAGRUN, AI.DAGRUN_RO ) * [add] pytorch oss cluster tests passing * [add] TensorFlow lite tests passing on oss cluster * [add] ONNX tests passing on oss cluster * [add] TensorFlow tests passing on oss cluster * [wip] wip on dag * [add] DAG tests passing on oss cluster * [add] enabling oss cluster tests on CI * [add] bumping rmbuilder version from 6.0.1 to 6.0.5 on circleci * [fix] fixed potential invalid mem accesses on RedisAI_DagRunSyntaxParser, RAI_parseDAGPersistArgs * [fix] fixed RAI_FreeDagOp wrongfully calling RedisModule_Free on dagOp->inkeys, dagOp->outkeys * [fix] fixed dictKey allocation on RAI_parseDAGLoadArgs * [add] alter Sanitizer tests to accommodate keys on the same slot. * [add] extracted the methods to respond to RedisModule_IsKeysPositionRequest() from main code of dagrun,modelrun, and scriptrun to specific methods * [fix] Fixes per PR review * [fix] fixes per PR review * [fix] fix per CI ascii conversion error on test_dagro_modelrun_financialNet_no_writes_multiple_modelruns * Safely add to arrays + fix for #443 (#449) * Make sure we reassign the pointer in array_append * Fix case-sensitive comparison for devicestr * Fix sanitizer tests (cherry picked from commit 5c7813e) * fixed tests messages * Shallow copy persisted tensor * Resolve log format warnings * Fix error message and test * Fix memory management in local context dict * Fixed artifacts handling + added tests logs aggregation (#445) * Snapshot packages are placed into http://redismodules.s3.amazonaws.com/redisai/snapshots. Private branches (i.e., non-master) are also supported, just need to enable deploy-snapshot in config.yml to fire on on-any-branch rather than just on-master. * Release packages are placed into http://redismodules.s3.amazonaws.com/redisai, and it's going to be crowded in there, so we may want to consider putting each version into its own directory. * `BB` in-source breakpoints are now supported (in `DEBUG=1` builds): put `BB;` inside the code and it will stop if you're running under gdb. * Tests results in CircleCI are now aggregated as artifacts. * Build: setuptools-related fix (#455) (cherry picked from commit d1fcd0d) Co-authored-by: Rafi Einstein <[email protected]> Co-authored-by: Luca Antiga <[email protected]> Co-authored-by: DvirDukhan <[email protected]> Co-authored-by: Guy Korland <[email protected]> Co-authored-by: Sherin Thomas <[email protected]>
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.
Fixes #401
TORCH and TFLITE didn't have the issue, while ONNX and TF did.
It might also be a good idea to add a test targeting this use case (not with full Yolo to avoid including a 200+MB model, only with a tiny model simulating a 1-batch input and 3-batch output).