-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Fixing unintentional variable overloading #14438
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the fix and welcome to mxnet. would you mind also adding an entry for yourself in https://github.com/apache/incubator-mxnet/blob/master/CONTRIBUTORS.md#list-of-contributors
Done, thanks! |
@mxnet-label-bot add [Example, Bug, Python, pr-awaiting-testing] |
@zjost Thank you for your contribution! Could you please look into the CI test failure? Thanks! |
Looks like something related to a clojure artifact. This should have nothing to do with my changes. |
@zjost Cool trick: If you edit the PR description and add Your PR is breaking on the clojure test due to an unrelated issue (#14448). You should rebase your commit onto master (git pull --rebase) so the fix to that issue is applied to your commit. It should pass the CI after that |
|
@zjost you can push an empty commit to retrigger the test. also can you rebase the latest master onto your branch. |
…o recommender-ex-fix rebase merge
If the code leading to the test failures is already in master, what's the harm of merging my rather innocuous changes to the example? Presumably the code I'm needing to rebase against is failing the same tests. |
* Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list
Sweet, thanks! |
@zjost thanks for the patch! |
* Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list
* Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list * Fixing unintentional variable overloading * Adding to conbributor list
Description
In the recommender example, the for loop uses
i
for enumeration, but theni
is also used within the loop for list comprehensions, which breaks their utility. Instead of being an integer,i
becomes an mx.nd.arrayresolves #14439
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
i
toidx
in loop withinmatrix_fact.evaluate_network
i
toidx
in loop withinmatrix_fact.train
Comments