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

Dgl ops 2 #16416

Merged
merged 12 commits into from
Oct 23, 2019
Merged

Dgl ops 2 #16416

merged 12 commits into from
Oct 23, 2019

Conversation

ChaiBapchya
Copy link
Contributor

@ChaiBapchya ChaiBapchya commented Oct 9, 2019

Description

Added tests for following ops (required by DGL)
Input - Large tensor array
Hyperbolic ops

  • arccosh
  • arcsinh
  • arctanh
  • cosh
  • sinh
  • tanh

Misc

  • sign
  • batch_dot
  • divide

Logical Ops

  • logical_and
  • logical_not
  • logical_or
  • logical_xor

Regression

  • Linear Reg
  • Logistic Reg

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Add tests to test_large_array.py

tests/nightly/test_large_array.py Outdated Show resolved Hide resolved
tests/nightly/test_large_array.py Outdated Show resolved Hide resolved
@sxjscience
Copy link
Member

sxjscience commented Oct 15, 2019

I think we've tested for batch_dot before

Okay, this is specific to large array.

@ChaiBapchya
Copy link
Contributor Author

Batch_dot operator hasn't been tested for large array before. Atleast not in the test_large_array.py file

Copy link
Member

@anirudh2290 anirudh2290 left a comment

Choose a reason for hiding this comment

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

The code for the current PR and https://github.com/apache/incubator-mxnet/pull/16497/files looks very similar. What is the reason for two different PRs ?

tests/nightly/test_large_array.py Show resolved Hide resolved
tests/nightly/test_large_array.py Outdated Show resolved Hide resolved
@ChaiBapchya
Copy link
Contributor Author

@anirudh2290 It looks same coz we are testing same functions. Agreed that we can bring both (large array input dim >=2) and large vector under same file. But for now going with this. Adding "merging array & vector tests" to the todo.

@access2rohit
Copy link
Contributor

@ChaiBapchya can you restart sanity test on your PR

Copy link
Member

@anirudh2290 anirudh2290 left a comment

Choose a reason for hiding this comment

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

Please address the two comments and add test results on the PR. LGTM otherwise.

@anirudhacharya anirudhacharya merged commit 10941ab into apache:master Oct 23, 2019
@ChaiBapchya ChaiBapchya deleted the dgl_ops_2 branch October 23, 2019 00:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants