You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2018/06/15 18:30:42 UTC

[GitHub] ThomasDelteil commented on issue #11171: Flaky test: test_gluon.test_hybrid_static_memory_switching

ThomasDelteil commented on issue #11171: Flaky test: test_gluon.test_hybrid_static_memory_switching
URL: https://github.com/apache/incubator-mxnet/issues/11171#issuecomment-397706460
 
 
   @piiswrong you introduced this test in this commit 
   
   [WIP] Do Not Merge. Static memory allocation for cached_op (#10817) https://github.com/apache/incubator-mxnet/commit/2dbd143e4892bb9ad4aa1835c79f0046603e3531
   
   and it seems to be flaky. I have seen it failing a few times in recent builds. Can you take a look? e.g 
   http://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/incubator-mxnet/detail/master/1002/pipeline/
   
   Given all the talk about quality contributions going on the dev mailing list, I am a little unsettled by the fact this PR went undocumented (no design docs or explanation in the PR), unreviewed (1 question ignored), the optimization wasn't tested or benchmarked (it was actually making slower), and the code was self-merged.
   
   Should we enforce that enforce that each PR , especially the ones that introduce a significant number of changes be properly documented and reviewed before merging?
   
   @szha @marcoabreu 

----------------------------------------------------------------
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