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

Fix symbolic example for rand_zipfian function #13855

Closed

Conversation

ChaiBapchya
Copy link
Contributor

Description

mx.sym.contrib.rand_zipfian function example fixed
It contained NDArray specific example for a Symbolic API

@ChaiBapchya ChaiBapchya requested a review from szha as a code owner January 11, 2019 23:05
@anirudhacharya
Copy link
Member

@mxnet-label-bot add [pr-awaiting-review]

@marcoabreu marcoabreu added the pr-awaiting-review PR is waiting for code review label Jan 14, 2019
@KellenSunderland
Copy link
Contributor

LGTM but I can't tell what's fixed. Were the values reported in the docs wrong?

@ChaiBapchya
Copy link
Contributor Author

The example before had NDArray. I replaced it with a corresponding Symbolic example

@stu1130
Copy link
Contributor

stu1130 commented Jan 17, 2019

@mxnet-label-bot update [pr-awaiting-review]
@KellenSunderland I think what he fixed is just doc. The symbol API should use smybol for demonstration rather than NDArray. And we won't get intermediate result with Symbol.

@marcoabreu marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge pr-awaiting-review PR is waiting for code review and removed pr-awaiting-review PR is waiting for code review pr-awaiting-merge Review and CI is complete. Ready to Merge labels Jan 17, 2019
@KellenSunderland
Copy link
Contributor

Gotcha. Lgtm.

@ChaiBapchya
Copy link
Contributor Author

@safrooze @ThomasDelteil Do you mind considering my PR since I had discovered it before and made PR as well?

@ChaiBapchya
Copy link
Contributor Author

Never mind. Other PR #13978 Got merged into the repo (despite being a duplicate of this PR).
Anyway, closing this PR.

@ChaiBapchya ChaiBapchya deleted the symbol_rand_zipfian_eg_fix branch January 25, 2019 22:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants