[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-392412662
 
 
   I think they were originally introduced to test int32 overflow, but I
   totally agree that these are suboptimal and should be rather be mocked in
   the C++ tests opposed to actually allocating so much memory.
   
   Da Zheng  schrieb am Mo., 28. Mai 2018, 04:35:
   
   > @marcoabreu  I think we should remove the
   > tests for generating huge weight matrices, as @juliusshufan
   >  is doing right now for Dense.
   > It's clear that the code will run into out-of-memory errors, regardless of
   > the backend (native MXNet, MKLDNN or CuDNN).
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > 
,
   > or mute the thread
   > 

   > .
   >
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-392219240
 
 
   Aaaah no no, definitely not. It was just a general statement :)
   
   Da Zheng  schrieb am Sa., 26. Mai 2018, 02:14:
   
   > @marcoabreu  i'm not sure what you mean.
   > do you mean we should support NDArrays of 85GB? i don't think we can use
   > malloc to allocate a single piece of memory of this size?
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > 
,
   > or mute the thread
   > 

   > .
   >
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-392105517
 
 
   I think it's a good these tests failed - it means that we actually have some 
issues and are being inconsistent. That's a great point to start from.
   
   I'd appreciate it if you could make the necessary changes in order to make 
all parts consistent. 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-390222345
 
 
   Gluon fits even better, good idea!
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-389990597
 
 
   Thanks a lot for adding so many tests, it's very appreciated!!!


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-388637287
 
 
   I see, thanks a lot for elaborating! Does this mean that MKLDNN is only 
being used if the input is in a certain shape? What happens in unusual shapes? 
The problem with this test is that you are not able to verify whether MKLDNN 
has actually been hit or if another implementation was used, right?
   
   We might have to start looking into CPP tests for MKLDNN specific tasks, 
considering these backends are designed to be transparent and a lot of 
information is abstracted away due to the hourglass C-API. It might be easier 
to validate your assumptions in CPP. What do you think? I don't feel strongly 
about it, but this is something we should look into in future.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-388618909
 
 
   Hi, thanks for adding these tests! Could you elaborate why we need backend 
specific tests in a front-end language for operators? Please excuse me if I'm 
making a misassumption here, but the implementation should be transparent and 
always act the same.
   
   This means we should not need any MKL specific tests for operators, 
considering their behaviour should be identical. From my point of view, there 
should rather be general tests for all operators (in the same style you just 
wrote them), and they should just succeed if we switch the backend to MKL. 
   
   I'm afraid that we will run into inconsistencies if we start writing custom 
tests for each backend. Ideally, all backends should produce the exact same 
output and execute the same behaviour. This would then be verified with general 
operator tests and thus make custom backend tests obsolete
   
   @szha @piiswrong @zheng-da am I right with my assumption?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon

[GitBox]

marcoabreu commented on issue #10921: Test cases improvement for MKLDNN on Gluon
URL: https://github.com/apache/incubator-mxnet/pull/10921#issuecomment-388618909
 
 
   Hi, thanks for adding these tests! Could you elaborate why we need backend 
specific tests in a front-end language? Please excuse me if I'm making a 
misassumption here, but the implementation should be transparent and always act 
the same.
   
   This means we should not need any MKL specific tests for operators, 
considering their behaviour should be identical. From my point of view, there 
should rather be general tests for all operators (in the same style you just 
wrote them), and they should just succeed if we switch the backend to MKL. 
   
   I'm afraid that we will run into inconsistencies if we start writing custom 
tests for each backend. Ideally, all backends should produce the exact same 
output and execute the same behaviour. This would then be verified with general 
operator tests and thus make custom backend tests obsolete
   
   @szha @piiswrong @zheng-da am I right with my assumption?


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:
us...@infra.apache.org


With regards,
Apache Git Services