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 2020/12/01 09:12:52 UTC

[GitHub] [incubator-mxnet] PawelGlomski-Intel opened a new pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

PawelGlomski-Intel opened a new pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608


   `print(mx.nd.arange(1, 5, dtype=[('bfloat16', np.uint16)]))`
   
   Before: `[(16256,) (16384,) (16448,) (16512,)]`
   After: `[1. 2. 3. 4.]`
   
   Resolves #18788
   


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



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-737079281


   Jenkins CI successfully triggered : [clang, edge, centos-cpu, sanity, miscellaneous, centos-gpu, unix-cpu, website, unix-gpu, windows-gpu, windows-cpu]


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



[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-738648171


   Yes, sorry, here is part of our discussion on this:
   
   > I was looking for a solution that doesn't require copying, and Andrzej's solution of setting numpy's formatter seems to be the only way to achieve this, but there are 2 problems.
   > 
   > The first one is that the user cannot change the way floats are printed, as numpy does not format scalars (the lambda we provide to the formatter takes scalar as argument). There is a hack to overcome this problem: putting each element into a 1-element ndarray and removing '\[' and '\]' from its string representation, but because now each element is treated separately, each may be displayed in a different form (chosen by numpy), which differs from the way numpy prints ndarrays - using only one notation.
   > 
   > The second problem is the performance. Printing bigger arrays is significantly slower using this solution, as we are doing computation on the python side (changing from bfloat16 to float32).
   > 
   > In the end, I chose Bartłomiej's solution, as it was easy to understand, consistent with numpy and fast.
   
   Where Bartłomiej's solution is the one proposed in this PR, and here is the code of the other solution (without copying):
   ```
       def __str__(self):
           if self.dtype == np.dtype([('bfloat16', np.uint16)]):
               def castToBFloat16(value):
                   return struct.unpack("@f", struct.pack("@I", np.uint32(value) << 16))[0]
               po = np.get_printoptions()
               po['formatter'] = {} 
               po['formatter']['int_kind'] = lambda x: str(np.array([castToBFloat16(x)]))[1:-1]
               with np.printoptions(**po):
                   return super(NDArray, self).__str__()
           else:
               return super(NDArray, self).__str__()
   ```


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



[GitHub] [incubator-mxnet] PawelGlomski-Intel edited a comment on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel edited a comment on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-738648171


   Yes, sorry, here is part of our discussion on this:
   
   > I was looking for a solution that doesn't require copying, and the solution of setting numpy's formatter seems to be the only way to achieve this, but there are 2 problems.
   > 
   > The first one is that the user cannot change the way floats are printed, as numpy does not format scalars (the lambda we provide to the formatter takes scalar as argument). There is a hack to overcome this problem: putting each element into a 1-element ndarray and removing '\[' and '\]' from its string representation, but because now each element is treated separately, each may be displayed in a different form (chosen by numpy), which differs from the way numpy prints ndarrays - using only one notation.
   > 
   > The second problem is the performance. Printing bigger arrays is significantly slower using this solution, as we are doing computation on the python side (changing from bfloat16 to float32).
   > 
   > In the end, I chose the solution with copying, as it was easy to understand, consistent with numpy and fast.
   
   Here is the solution without copying which I am referring to above:
   ```
       def __str__(self):
           if self.dtype == np.dtype([('bfloat16', np.uint16)]):
               def castToBFloat16(value):
                   return struct.unpack("@f", struct.pack("@I", np.uint32(value) << 16))[0]
               po = np.get_printoptions()
               po['formatter'] = {} 
               po['formatter']['int_kind'] = lambda x: str(np.array([castToBFloat16(x)]))[1:-1]
               with np.printoptions(**po):
                   return super(NDArray, self).__str__()
           else:
               return super(NDArray, self).__str__()
   ```


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



[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-737074747


   @mxnet-bot run ci [greeting]


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



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-736332757


   Hey @PawelGlomski-Intel , Thanks for submitting the PR 
   All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands: 
   - To trigger all jobs: @mxnet-bot run ci [all] 
   - To trigger specific jobs: @mxnet-bot run ci [job1, job2] 
   *** 
   **CI supported jobs**: [clang, centos-cpu, sanity, unix-gpu, windows-cpu, website, centos-gpu, unix-cpu, miscellaneous, windows-gpu, edge]
   *** 
   _Note_: 
    Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin. 
   All CI tests must pass before the PR can be merged. 
   


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



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-744238901


   Jenkins CI successfully triggered : [edge, centos-gpu]


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



[GitHub] [incubator-mxnet] leezu merged pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
leezu merged pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608


   


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



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-737074783


   None of the jobs entered are supported. 
   Jobs entered by user: [greeting]
   CI supported Jobs: [clang, website, miscellaneous, sanity, unix-gpu, centos-gpu, centos-cpu, windows-gpu, unix-cpu, windows-cpu, edge]
   


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



[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-744238832


   @mxnet-bot run ci [centos-gpu, edge]


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



[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #19608: Change __str__ representation of bfloat16 NDArray from uints to floats.

Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #19608:
URL: https://github.com/apache/incubator-mxnet/pull/19608#issuecomment-737079204


   @mxnet-bot run ci [all]


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