[Clojure] Add methods based on NDArrayAPI/SymbolAPI

[kedarbellare]

Description

• This PR generates new methods based on the newer NDArrayAPI and SymbolAPI.
• It uses the scala's Base/_LIB with direct access to JNI in order to generate the ndarray and symbol methods along with named arguments and documentation. Although the current PR doesn't add specs these can be added over time.

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

• NDArrayAPI methods, tests, (and when applicable, API doc)
• SymbolAPI methods, tests, (and when applicable, API doc)

Reviewers

@gigasquid

[roywei]

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

[gigasquid]

Thanks for looking into this. It will be interesting to see how it comes out. The new API might be more difficult to interop with than the regular data structures of the current one.

[kedarbellare]

@gigasquid it does seem like the new API is somewhat more cumbersome in terms of interop. the main benefit i see is the type checking but while in scala you can call the functions with named optional arguments (e.g. Dropout(ndarray, p = Some(0.25), out = Some(outputArray))) it's not so straightforward to have this in clojure.

[gigasquid]

@kedarbellare Thanks so much for looking into this. I thought it might be the case that it might not be as nice to use. That's totally fine. We can stick to the other API for now and if for some reason we need to move off of it (like the Scala package deprecates it) , we can think of a different strategy, like moving a level down for NDArrary and Symbol and using the Scala Generator functions to generate our own functions directly using the JNI calls instead.

For example - getting all the backend functions by calling the JNI
https://github.com/apache/incubator-mxnet/blob/master/scala-package/macros/src/main/scala/org/apache/mxnet/GeneratorBase.scala#L63

and these to create the functions
https://github.com/apache/incubator-mxnet/blob/master/scala-package/macros/src/main/scala/org/apache/mxnet/GeneratorBase.scala#L73

Let me know what you think.

[kedarbellare]

Makes sense. I'm working on unit tests for NDArray API right now which should give an idea of how we could start using it. I won't put too much effort into doing the same for Symbol API. It'll be great if we can start using JNI directly but I don't have much experience on that front.

[gigasquid]

@kedarbellare Thanks for tackling the toughest part of the whole Clojure package 💯 I think you have made some really nice progress and we can definitely leverage some of this work.

Since we are starting a new ndarray api, we have a nice opportunity to improve upon the current api and make it more useful for Clojure users. I agree with your assessment that the typing makes things better, but all the positional arguments and options make it more cumbersome. This is due to the philosophical differences between Clojure and Scala, where Clojure loves data structures and Scala loves types :) I'm thinking we should take a step back and think about what we (the clojure package) would want and then try to shape what you have into it. After looking at what you have, I think there is a path forward with maybe generating only the arity with all the parameters and then taking a data structure of vectors or maps to get there.

I think it would be nice to outline what that API should look like with a couple examples from the NDArray and Symbol API before we code it up too much. Since you have experience and good feeling for what is possible now with your work, are you up for helping to draft up the future look of the API on the wiki?

[kedarbellare]

Yep. I wholeheartedly agree about the philosophical differences between Scala and Clojure and I completely see the value in doing what's best for the Clojure package. I can try to take a stab at creating an outline so that we can add it to the wiki and ask for comments. Note, however, that I mostly work on this project during the weekends or when I have free time so I'll try my best to put something together soon (over the next week or so). Please let me know if you want to have something out there ASAP, in which case it may not make sense for me to work on it.

[gigasquid]

@kedarbellare Definitely no rush - whatever pace works well for you. I appreciate your efforts and your contributions to improve the Clojure project.

I don't have rights to create accounts on the wiki so I'll ask out on dev for access for you.

Thanks again!

[gigasquid]

@kedarbellare You should have access now - ping me if you have any trouble 😸

[vandanavk]

@mxnet-label-bot update [Clojure, pr-work-in-progress]

[kedarbellare]

@gigasquid i've improved the api generation for both ndarrays and symbols. PTAL thanks!

[gigasquid]

Awesome work!!! I look forward to reviewing this 😸

[Chouffe]

Amazing work Kedar!
I left some minor comments. I will review more in depth if needed. @gigasquid will have better feedback for you :)
Looking forward to having this merged!

[gigasquid]

Thanks @kedarbellare for tackling this and making such great progress!

After @Chouffe 's feedback is taken care of, I think this is fine to merge and will give us a good base to work on.

[gigasquid]

@kedarbellare - I think the next step after this is to try to get https://github.com/apache/incubator-mxnet/blob/master/contrib/clojure-package/test/org/apache/clojure_mxnet/conv_test.clj to use the new symbol-api. It will probably shake out some further refinements.

It certainly is a much nicer api to use and would love to get it to the point were we could deprecate the old symbol and ndarray way of doing things 😸

[gigasquid]

I'm also not sure why the unix-gpu keeps failing from CI, might want to try to rebase on master and see if that helps

[gigasquid]

Oh @kedarbellare - one more thing... Would you mind marking the new api namespaces as Experimental ? That way users know that it is in active work and might change and not be stable yet.

Thanks!

[kedarbellare]

@Chouffe @gigasquid : thanks for the detailed feedback! i'll take care of these issues soon (hopefully by end of the week).

@gigasquid : where should i mark the namespaces as Experimental? in the docs at the top or in the individual docs of the functions or somewhere else?

[gigasquid]

@kedarbellare marking it as Experimental in the namespace docstring of contrib/clojure-package/src/org/apache/clojure_mxnet/ndarray_api.clj - or a comment should be fine. And in the corresponding symbol-api namespace. Thanks!

[Chouffe]

We should probably start rewriting some of the examples with this once it gets merged.
I can help with this @kedarbellare @gigasquid :)

[kedarbellare]

@gigasquid @Chouffe i've updated the PR with the following:

1. addressed cleanup comments
2. tagged the new apis as "Experimental"
3. modified the conv_test.clj to use the new symbol-api and fixed a small bug that i discovered during that
4. tweaked the coerce-param for symbol-api (especially for the attr param)
5. rebased to the latest upstream/master to see if that fixes the unix-gpu CI failure

PTAL thanks!