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/07/01 09:43:44 UTC
[GitHub] [incubator-mxnet] BenjaminCHEN2016 opened a new pull request #18649: backport mixed type to 1.7
BenjaminCHEN2016 opened a new pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649
## Description ##
Backport mixed type binary ops to v1.7.x branch #18641
## Checklist ##
### Essentials ###
Please feel free to remove inapplicable items for your PR.
- [ ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) created (except PRs with tiny changes)
- [ ] Changes are complete (i.e. I finished coding on this PR)
- [ ] All changes have test coverage:
- Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
- Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
- Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
- [ ] Code is well-documented:
- For user-facing API changes, API doc string has been updated.
- For new C++ functions in header files, their functionalities and arguments are documented.
- For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
- Check the API doc at https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
- [ ] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
### Changes ###
- [ ] Feature1, tests, (and when applicable, API doc)
- [ ] Feature2, tests, (and when applicable, API doc)
## Comments ##
- If this change is a backward incompatible change, why must this change be made.
- Interesting edge cases to note here
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653393406
@leezu Is the CI different between master and v1.7.x branch? It seems that windows CI on v1.7.x is an older version?
----------------------------------------------------------------
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 #18649: backport mixed type to 1.7
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-652313463
Hey @BenjaminCHEN2016 , 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**: [centos-cpu, website, windows-cpu, unix-gpu, windows-gpu, edge, miscellaneous, unix-cpu, centos-gpu, clang, sanity]
***
_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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-698664200
If I remembered correctly, this PR only contains a minimum bug fix that are required for v1.7 other than the full features/bug fixes from master branch. If this is the case, then it might be better to try to pull back the full version of fix into v1.x as well as v1.8.x. Ping @BenjaminCHEN2016 to help confirm.
----------------------------------------------------------------
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] DickJC123 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
DickJC123 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-698652718
A person might reasonably assume that v1.8 == v1.7 plus select commits from the 1.x branch. Will this commit be added to v1.x and v1.8, or will v1.8 and other future 1.x releases be missing this functionality?
I'm basically trying to cherry-pick commits on top of my "v1.7-ish" repo to get to v1.8. Do you think I should revert this commit on my repo to minimize future integration conflicts?
----------------------------------------------------------------
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 #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653757513
Jenkins CI successfully triggered : [unix-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] szha merged pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
szha merged pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649
----------------------------------------------------------------
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] DickJC123 edited a comment on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
DickJC123 edited a comment on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-698652718
A person might reasonably assume that v1.8 == v1.7 plus select commits from the 1.x branch. Will this PR be added to v1.x and v1.8, or will v1.8 and other future 1.x releases be missing this functionality?
I'm basically trying to cherry-pick commits on top of my "v1.7-ish" repo to get to v1.8. Do you think I should revert this PR commit on my repo to minimize future integration conflicts?
----------------------------------------------------------------
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] samskalicky commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
samskalicky commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-695090455
@ciyongch @szha any reason this wasnt committed to 1.x branch?
----------------------------------------------------------------
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] szha commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653663802
Yes, we still need the change. It sounds like we may have missed some windows CI changes to backport. cc @ChaiBapchya to help clarify issues with windows CI
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653339521
@ciyongch I will try to resolve this issue today.
----------------------------------------------------------------
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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653845257
Thanks a lot @BenjaminCHEN2016 for your effort to make this patch pass all the CI tests. So we already have all we need for 1.7 release right now.
@leezu @ChaiBapchya @sxjscience @szha @TaoLv @pengzhao-intel please help to review and merge, then I will trigger the nightly test and prepare for rc0. Thanks!
----------------------------------------------------------------
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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-698664200
If I remembered correctly, this PR only contains a minimum bug fix that are required for v1.7 other than the full features/bug fixes from master branch. If this is the case, then it might be better to try to pull back the full version of fix into v1.x as well as v1.8.x. Ping @BenjaminCHEN2016 to help confirm.
----------------------------------------------------------------
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] wkcn commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
wkcn commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653411128
Hi @ciyongch , the issue is related to the version of compiler.
I only build MXNet on gcc6, gcc7 and gcc10, and gcc10 takes more than 11GB memory.
If using visual studio, could you try add `/Zm100 /GX` into the additional options of `Property/ C/C++ /All Options` to enlarge the heap space ?
----------------------------------------------------------------
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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653279462
@BenjaminCHEN2016 @sxjscience there's some build error on windows platform as below, please help to take a look. It'll be great if it can be solved within 24h, Thanks a lot!
```
[2020-07-02T13:28:03.699Z] c:\jenkins_slave\workspace\build-gpu\src\operator\tensor\elemwise_binary_broadcast_op.h(237) : fatal error C1002: compiler is out of heap space in pass 2
[2020-07-02T13:28:03.699Z] jom: C:\jenkins_slave\workspace\build-gpu\build\CMakeFiles\mxnet_52.dir\build.make [CMakeFiles\mxnet_52.dir\src\operator\numpy\np_elemwise_broadcast_op.cc.obj] Error 1
```
----------------------------------------------------------------
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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653369808
Hi @BenjaminCHEN2016 , it looks more like a compilation error which probably introduced by the current complex expressions or something like that which ran out of memory instead of the compiler itself.
I remembered there's a discussion about the compilation memory usage [here](https://github.com/apache/incubator-mxnet/issues/18501#issuecomment-641669164), which reported that several numpy operator files consume a large memory footprint during compilation.
But I'm curious if this is the case, why the master branch doesn't have the similar CI issue. Is there any additional or different change in this patch compared to the original PRs?
Ping @wkcn, @leezu to see if any suggestions for this case, thanks!
----------------------------------------------------------------
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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-652955823
Thank you @BenjaminCHEN2016 to backport these fixes. If I understand correctly, this is the only remaining PR that fix the numpy operator and targeting to 1.7 release, am I right? @sxjscience
Can you also help to test the latest v1.7.x code base with this PR in advance to make sure it's working as expected, and no more other dependencies? Thanks!
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-652887518
@sxjscience @ciyongch
----------------------------------------------------------------
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] DickJC123 edited a comment on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
DickJC123 edited a comment on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-698652718
A person might reasonably assume that v1.8 == v1.7 plus select commits from the 1.x branch. Will this PR be added to v1.x and v1.8, or will v1.8 and other future 1.x releases be missing this functionality?
I'm basically trying to cherry-pick commits on top of my "v1.7-ish" repo to get to v1.8. Do you think I should revert this PR commit on my repo to minimize future integration conflicts?
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653744700
@mxnet-bot run ci [unix-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] ChaiBapchya commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653667497
Windows CI issues in master branch in late March were fixed by PRs :
1. https://github.com/apache/incubator-mxnet/pull/17962
- Upgrade VS from 2015 to 2019
- use pre-installed cmake 3.17 [instead of installing 3.16]
- upgrade from 32bit to 64bit toolchain
- Cuda 10.2 [VS2019 requires 10.2]
2. https://github.com/apache/incubator-mxnet/pull/18177
- Fail build if windows retries fail
3. https://github.com/apache/incubator-mxnet/pull/18230
- Windows Cuda [thrust issue]
Infrastructure-wise, all branches run on same infra [meaning Same AMI, Same instance types, etc]
Code-wise I'm not sure if these PRs have been cherry-picked into other branches [specifically 1.7.x in this case].
Having said that, looking at past 15 CI runs on merged commits in 1.7.x [not 1 has failed on windows-cpu or windows-gpu].
@leezu any thoughts on those backports of your Windows CI specific fixes [that have been targeted towards master in the above mentioned PRs]?
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653357969
I think that might be a compiler issue? I tried on my windows machine and it can be compiled. @ciyongch @sxjscience
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653740855
@ChaiBapchya Thanks!
I applied the #17962 to bump the windows compiler to 64 bits. And, now the CI is working as expected.
@sxjscience @ciyongch I think it is ready to be merged now.
----------------------------------------------------------------
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] DickJC123 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
DickJC123 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-698652718
A person might reasonably assume that v1.8 == v1.7 plus select commits from the 1.x branch. Will this commit be added to v1.x and v1.8, or will v1.8 and other future 1.x releases be missing this functionality?
I'm basically trying to cherry-pick commits on top of my "v1.7-ish" repo to get to v1.8. Do you think I should revert this commit on my repo to minimize future integration conflicts?
----------------------------------------------------------------
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 #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653744713
Jenkins CI successfully triggered : [unix-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] szha commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-695093623
I think it was just missed.
----------------------------------------------------------------
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] BenjaminCHEN2016 commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
BenjaminCHEN2016 commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653757505
@mxnet-bot run ci [unix-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] ciyongch commented on pull request #18649: [v1.7.x] backport mixed type binary ops to v1.7.x
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18649:
URL: https://github.com/apache/incubator-mxnet/pull/18649#issuecomment-653418261
Thanks for your information @wkcn , according to @BenjaminCHEN2016 the failure only happened in MXNet windows CI pipeline but not his local environment. There's a concern that if this is the case and still not able to pass the CI, do we still need to include this in v1.7.0? @sandeep-krishnamurthy @szha @sxjscience .
----------------------------------------------------------------
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