D-Roberts commented on pull request #18757:
URL: https://github.com/apache/incubator-mxnet/pull/18757#issuecomment-661264530


   Hi @leezu .
   By 're-verify tests' I really meant that I ran the tests >3 times with 
MXNET_TEST_COUNT=10000 .
   
   I also made the following changes to the tests:
   1. I removed the expensive analytical and numerical Jacobian calculations 
that were included in the initial PR. These calculations were initially 
included to demonstrate that the analytical method that I was implementing was 
working correctly and passing the central differences numerical checks. At the 
time of the initial PR 3 months ago, MXNet was the only framework implementing 
this qr backward method for wide inputs. However, in the meanwhile (over the 
past 3 months), TensorFlow had this method implemented, so the method is 
already verified via central differences. Notice in the example given in the 
description that the results from MXNet implementation and Tf implementation 
are identical. 
   
   2. I included a larger matrix in the test input for a test case. I removed a 
couple of redundant test cases to reduce test run time. I also relaxed atol and 
rtol for dtype float32 to make sure there are no flaky test runs at any time 
due to numerical rounding errors.
   
   The error that led to the "stale PR" failing CI was due to a calculation in 
the numerical Jacobian for central differences check. The code broke after 
updates to Numpy and MXNet Numpy when the source array is float32 and the the 
dtype used in view is float64.
   ```
   "jacobian_num[row, :] = diff.asnumpy().ravel().view(dtype)
   [2020-07-17T17:41:26.700Z] E           ValueError: When changing to a larger 
dtype, its size must be a divisor of the total size in bytes of the last axis 
of the array.
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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


Reply via email to