-
Notifications
You must be signed in to change notification settings - Fork 836
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
Add micro-batching to batch processor #3301
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
tensor_ndarray = tensor.reshape(shape) | ||
|
||
for i in range(len(input_data)): | ||
new_response = copy.deepcopy(response) |
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.
How inefficient is this copy for large batch size?
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.
I believe it should have negligible impact when considering we still need to read and write each row separately to the file, but I agree it's not ideal.
We need to split the single response into X responses which share 90% of the same content. I can't think of a more efficient way to do it off the top of my head?
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.
Why do you need to deepcopy anyway? Can't you just save 'response["data"]["ndarray"]' as a variable and then always just override the original value? Eventually you dump them into strings so overriding won't really "ruin" the ones you are already done with, right?
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.
Yeah this is a good idea, would you like to raise a PR for it @naor2013? If not I can address at a later date.
@majolo Looks good. Has just 1 question. |
/test integration |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cliveseldon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Wow I didn't expect this to merge yet... |
Production Python a bit rusty so glad to hear constructive comments.
Fixes #2734
Benefits: