[GitHub] [incubator-mxnet] cjolivier01 commented on issue #15167: Pointwise fusion for GPU
cjolivier01 commented on issue #15167: Pointwise fusion for GPU URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-529008979 I am removing my block on this PR and letting in this "auto-abuse" in since I am in a similar industry to NVidia and don't wish to make it look like I am blocking a performance improvement for NVidia GPU's. I hope that the auto-abuse will be fixed before merging, however. 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] cjolivier01 commented on issue #15167: Pointwise fusion for GPU
cjolivier01 commented on issue #15167: Pointwise fusion for GPU URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-516698343 Google c++ style guide says basically the same thing as what I’m encouraging: https://google.github.io/styleguide/cppguide.html#auto I’d like to review this PR, but it’s unreadable in its current state, imho. I do not intend to have extended arguments on a case-by-case basis, as I don’t think that sort of turnabout would ultimately be constructive. 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] cjolivier01 commented on issue #15167: Pointwise fusion for GPU
cjolivier01 commented on issue #15167: Pointwise fusion for GPU URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-514835143 1) i randomly picked that line as representative of auto used for pretty much everything 2) pointing out this or that line is “already in master” at some random place is interesting. If you got cancer, would you have the attitude like “I already have some cancer in me, so clearly more cancer is acceptable”, or would you try to get rid of the cancer if possible, or at the very least slow its growth? If it was a couple of autos here and there, it could pass, but this PR kind of takes that to another level and sets a bad precedent for future code (in case someone wants to point out, well they use “auto” everywhere in this code)... 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] cjolivier01 commented on issue #15167: Pointwise fusion for GPU
cjolivier01 commented on issue #15167: Pointwise fusion for GPU URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-514832116 mxnet doesn’t yet have a c# interface — are you volunteering? The functionality would be welcomed :) 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] cjolivier01 commented on issue #15167: Pointwise fusion for GPU
cjolivier01 commented on issue #15167: Pointwise fusion for GPU URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-514792263 We apparently have very different interpretation of those guidelines. I don’t see anything in that which implies “use it everywhere for everything”. I’m more of the opinion stated here: While I am not i. the habit of posting stack overflow nonsense as “evidence”, seems to be a consistent understanding about proper auto use: https://www.acodersjourney.com/c-11-auto/ https://stackoverflow.com/questions/6900459/the-new-keyword-auto-when-should-it-be-used-to-declare-a-variable-type Using auto everywhere makes C++ code no more readable than something like python. 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] cjolivier01 commented on issue #15167: Pointwise fusion for GPU
cjolivier01 commented on issue #15167: Pointwise fusion for GPU URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-514768835 I looked at those “core guildelines” and it’s basically what I was saying — stuff like ::iterator and repetition or obvious type on same line. where you get the “always use auto” I don’t know. from the video? videos are TLDR-ish, but I certainly wouldn’t trust anyone who makes a video saying to use auto everywhere. that’s just crazy :) 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 With regards, Apache Git Services
