Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

Fix nightly build#16773

Merged
marcoabreu merged 5 commits intoapache:masterfrom
hzfan:conf_pr
Nov 14, 2019
Merged

Fix nightly build#16773
marcoabreu merged 5 commits intoapache:masterfrom
hzfan:conf_pr

Conversation

@hzfan
Copy link
Contributor

@hzfan hzfan commented Nov 11, 2019

Description

Include tvmop.conf as binary dependencies.

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 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 my best 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

Thank @yzhliu for guidance and review.

@hzfan hzfan requested a review from szha as a code owner November 11, 2019 05:51
@hzfan
Copy link
Contributor Author

hzfan commented Nov 11, 2019

@mxnet-label-bot add [CI]

@lanking520 lanking520 added the CI label Nov 11, 2019
@hzfan hzfan mentioned this pull request Nov 11, 2019
Closed
@ChaiBapchya
Copy link
Contributor

ChaiBapchya commented Nov 11, 2019

Hey @hzfan thanks for the PR. Have we tested on CI-dev if it passes the nightly? That's generally a good indicator. I can do it for you if you haven't.

larroy
larroy reviewed Nov 11, 2019
View reviewed changes
python/mxnet/tvmop.py Outdated
try:
_CONF_TVM_OP = find_conf_path("tvmop")
except RuntimeError as e:
logging.warning(e)
Copy link
Contributor

@larroy larroy Nov 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add comments on what's the implication of this? Why we just print a warning. It's not very clear when going through the code.

Copy link
Contributor Author

@hzfan hzfan Nov 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments added. The config file records available schedules, but besides these, a default schedule is also available, so when the conf is missing, we can fallback to default.

Copy link
Contributor

@marcoabreu marcoabreu Nov 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you still add a message that explains this to the user? otherwise, they just see an error as warning without context.

@hzfan
Copy link
Contributor Author

hzfan commented Nov 12, 2019

Hey @hzfan thanks for the PR. Have we tested on CI-dev if it passes the nightly? That's generally a good indicator. I can do it for you if you haven't.

Hi @ChaiBapchya , haven't tested on CI-dev nightly yet. Appreciate it if if you could help.

@marcoabreu
Copy link
Contributor

marcoabreu commented Nov 12, 2019

Oups I tried to suggest a change, but instead directly committed to your branch. Sorry about that! I still have to figure out how the proposal systems works <.<

@ChaiBapchya
Copy link
Contributor

ChaiBapchya commented Nov 12, 2019 •
edited
Loading

Hey @hzfan thanks for the PR. Have we tested on CI-dev if it passes the nightly? That's generally a good indicator. I can do it for you if you haven't.

Hi @ChaiBapchya , haven't tested on CI-dev nightly yet. Appreciate it if if you could help.

Sure. I have pointed the NightlyTestForBinaries to test out for conf_pr branch in your repository.
Once it passes/fails, I will update the result here.

http://jenkins.mxnet-ci-dev.amazon-ml.com/blue/organizations/jenkins/NightlyTestsForBinaries/detail/conf_pr/1/pipeline

It passed.

samskalicky
samskalicky approved these changes Nov 14, 2019
View reviewed changes
Copy link
Contributor

@samskalicky samskalicky left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, now that the nightly passed (thanks @ChaiBapchya) lets merge this PR @reminisce @ptrendx

@marcoabreu marcoabreu merged commit 7e21bda into apache:master Nov 14, 2019
ptrendx pushed a commit to ptrendx/mxnet that referenced this pull request Nov 15, 2019
@hzfan @ptrendx
Fix nightly build (apache#16773)
73f8a22 
* Remove dependency on tvmop.conf

* Fix binaries dependencies for ni nightly

* Add comments

* Update tvmop.py

* Fix rebase
@ptrendx ptrendx mentioned this pull request Nov 15, 2019
Merged
ptrendx added a commit that referenced this pull request Nov 16, 2019
@ptrendx
Backport to 1.6 (#16773, #16781, #16783, #16716, #16699, #16728, #16769…
9abb151 
…, #16792) (#16832)

* Fix nightly build (#16773)

* Remove dependency on tvmop.conf

* Fix binaries dependencies for ni nightly

* Add comments

* Update tvmop.py

* Fix rebase

* Fix (#16781)

* Speed fused_op compilation by caching ptx and jit-compiled functions (#16783)

* [Numpy] Fix collect_params().zero_grad() in gluon numpy interface (#16716)

* fix zero_grad

* Update parameter.py

* add test

* fix

* Mixed data type binary ops (#16699)

* support mixed-precision binary operations

* improvement for documentations and error messages

* Support boolean elemwise/broadcast binary add, multiply and true_divide (#16728)

* support pure boolean elemwise/broadcast binary op

* switch to unique_tpr

* fix the test error

* Fix rtrue_divide grad (#16769)

* Fix rtrue_divide_scalar

* More tests

* Fix numpy-compatible mean output type for integer inputs (#16792)

* fix mean output type for integer inputs

* enable for windows
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@szha szha Awaiting requested review from szha szha is a code owner

4 more reviewers

@larroy larroy larroy left review comments

@marcoabreu marcoabreu marcoabreu left review comments

@samskalicky samskalicky samskalicky approved these changes

@reminisce reminisce reminisce approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@hzfan @ChaiBapchya @marcoabreu @larroy @samskalicky @reminisce @lanking520