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

Fix nightly build #16773

Merged
merged 5 commits into from
Nov 14, 2019
Merged

Fix nightly build #16773

merged 5 commits into from
Nov 14, 2019

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

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

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

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

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

@marcoabreu marcoabreu marcoabreu left review comments

@larroy larroy larroy left review comments

@reminisce reminisce reminisce approved these changes

@samskalicky samskalicky samskalicky approved these changes

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

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