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

Fix Clojure BERT example's context argument #14843

Merged
merged 3 commits into from
Apr 30, 2019

Conversation

daveliepmann
Copy link
Contributor

@daveliepmann daveliepmann commented Apr 30, 2019

Description

The Clojure BERT example's infer function accepts a CPU/GPU context, which the command line
version of this example exposes as a :cpu/:gpu keyword. Previously, these options were ignored and the context was always overridden to the default context (CPU). This PR allows users (both REPL and shell) to pass in a GPU context.

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 (this is a tiny-change PR)
  • Changes are complete (i.e. I finished coding on this PR)
  • 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

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

* Remove unused requires
* Remove unused vars & function
* Use `io` alias
The `infer` function accepts a CPU/GPU context, which the command line
version of this example exposes as a `:cpu`/`:gpu`
keyword. Previously, these options were ignored and the context was
overridden to the default context (CPU). This commit allows
users (both REPL and shell) to pass in a GPU context.
Copy link
Member

@gigasquid gigasquid left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes! I have a PR to include this example in a more general bert example. I'll make sure that I get your changes in there as well #14769

@gigasquid gigasquid merged commit cdd7087 into apache:master Apr 30, 2019
@daveliepmann daveliepmann deleted the fix-clojure-bert-context branch May 1, 2019 08:38
@daveliepmann
Copy link
Contributor Author

Thanks Carin. I had just seen your in-progress PR before this got merged. Sorry for not doing my due diligence and making my changes there.

access2rohit pushed a commit to access2rohit/incubator-mxnet that referenced this pull request May 14, 2019
* Clojure BERT example: minor code cleanup

* Remove unused requires
* Remove unused vars & function
* Use `io` alias

* Clojure BERT example: whitespace fix

* Clojure BERT example: allow running with GPU

The `infer` function accepts a CPU/GPU context, which the command line
version of this example exposes as a `:cpu`/`:gpu`
keyword. Previously, these options were ignored and the context was
overridden to the default context (CPU). This commit allows
users (both REPL and shell) to pass in a GPU context.
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* Clojure BERT example: minor code cleanup

* Remove unused requires
* Remove unused vars & function
* Use `io` alias

* Clojure BERT example: whitespace fix

* Clojure BERT example: allow running with GPU

The `infer` function accepts a CPU/GPU context, which the command line
version of this example exposes as a `:cpu`/`:gpu`
keyword. Previously, these options were ignored and the context was
overridden to the default context (CPU). This commit allows
users (both REPL and shell) to pass in a GPU context.
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.

None yet

2 participants