[GitHub] ThomasDelteil commented on issue #11318: Revert "Static alloc for hybridblock"

[GitBox <gi...@apache.org>]

ThomasDelteil commented on issue #11318: Revert "Static alloc for hybridblock"
URL: https://github.com/apache/incubator-mxnet/pull/11318#issuecomment-397778642
 
 
   I think I have raised specific concerns (twice) that have gone unanswered, just in case they got lost in the flow, let me write them here again:
   - "good changes", I'm sure they are, but the problem with the previous PR was that it was actually making things slower due to a bug, hopefully now fixed. I think we cannot just rely on blind trust, being data driven is beneficial for everybody. Could we have a runtime of maybe just a MNIST training to show that the speed is actually improved and rejoice accordingly? 
   - The new API says that static_shape cannot be used without static_alloc. Can we assert that static_alloc is set if users set static_shape to True to avoid undefined behavior?
   
   Bonus ask:
   - a description of the PR to help reviewers, and users alike to appreciate the changes and guide them through the 1400 lines of code, that have barely any comments.
   - better documentation on the impact of the bulk_size arguments in the hybridize() function

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services