Add MXNet backend #83
Conversation
Add a placeholder file for the MXNet backend and partially fill it in. In addition also add mxnet to packages to pip install and add mxnet_backend to the list of backends to test.
pyhf/tensor/mxnet_backend.py
Outdated
| """Backend for MXNet""" | ||
|
|
||
| def __init__(self, **kwargs): | ||
| self.session = kwargs.get('session') |
There was a problem hiding this comment.
seems like MXNet does not have a concept of a session (at least I can't find a reference to it. So I think this can be removed?
There was a problem hiding this comment.
Yeah, that was the result of some sloppy copypasta and me not checking things late at night. I'll fix that up when I get back to this after finishing work tonight.
matthewfeickert
changed the title
Other methods are added as well, but these are the two that were the most important
|
Sorry this is taking a bit of time. I have things mostly done but I was getting a bit confused with the way that MXNet handles things like this: https://discuss.mxnet.io/t/ndarray-conversion-from-other-array-types/258 I can probably push a working version later tonight once I finish some analysis work, but I canceled all the builds for the last push. |
pyhf/tensor/mxnet_backend.py
Outdated
|
|
||
| def poisson(self, n, lam): | ||
| pass | ||
| return self.normal(n, lam, self.sqrt(lam)) | ||
|
|
||
| def normal(self, x, mu, sigma): |
There was a problem hiding this comment.
You can probably use these APIs
https://mxnet.incubator.apache.org/api/python/ndarray/random.html
There was a problem hiding this comment.
but for poisson, let's stick for now to a gaussian approximation with mu = lambda , sigma = sqrt(lambda) since we need continuous poissons
There was a problem hiding this comment.
Hm, maybe I'm misunderstanding what you mean, but we want the pdf of the distribution evaluated at x, yes? The MXNet distribution generators return sampling of the distributions given the parameters you pass, so mx.nd.random.normal() is akin to np.random.normal(), and we want something like scipy.stats.norm.pdf(x), right? I might have missed another API though.
Yeah, I follow you RE: the Poissons.
There was a problem hiding this comment.
ah right, i didn't look to closely at the api seems like it more close to np.random stan scipy.stats.. sorry about that!
There was a problem hiding this comment.
No worries! Just wanted to make sure I wasn't being dumb. :)
The current outer() function is highly suboptimal. However, until a better solution can be implimented in a more MXNet-centric style it will at least pass the tests. The Pull Request in which this commit exists should _NOT_ be merged in until outer() is updated with a better solution.
Pull Request Test Coverage Report for Build 256
💛 - Coveralls |
|
@lukasheinrich Please don't accept this yet even though it passes. There are some things I want to fixup in it. |
Use the linear algebra functions of MXNet to provide a more clean implimenation of the outer product method, outer()
matthewfeickert
force-pushed
the
add-MXNet-backend
branch
from
e06bf7c
to
bf92334
Compare
Extended iterable unpacking wasn't added until PEP 3132 and so isn't in Python 2, which is causing the Python 2.7 tests to fail. While this is annoying, it is probably worth still supporting Python 2.7 until the official Python 2 end of life date. c.f. https://www.python.org/dev/peps/pep-3132/
matthewfeickert
force-pushed
the
add-MXNet-backend
branch
from
d99be1f
to
ced125f
Compare
To support the more complex systems of multiple backends a Conda environment.yml file is used instead of a pip requirements.txt file for use with Binder. c.f. https://mybinder.readthedocs.io/en/latest/sample_repos.html#conda-environment-with-environment-yml
matthewfeickert
force-pushed
the
add-MXNet-backend
branch
from
ced125f
to
7fc9d37
Compare
Additionally make a correction to outer(). nd.broadcast_mul() is the correct function to use in this method.
Add Google Style docstrings that should be compatible with Sphinx under certain themes, such as the Sphinx theme created by Read The Docs. For an example of what this would look like, see PyTorch's docs. c.f.: http://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google http://pytorch.org/docs/master/_modules/torch/distributions/bernoulli.html#Bernoulli
matthewfeickert
force-pushed
the
add-MXNet-backend
branch
from
7fc9d37
to
e40a007
Compare
|
@lukasheinrich I think this is ready for review, and if no comments then to be merged in. |
|
LGTM! thanks a lot! |
|
Closes Issue #80 |
Add in a backend for MXNet as described in Issue #80.
TODO:
mxnet_backend.pymethodsmxnetas possibleThis was made easier by the "PyTorch to MXNet" README from the online book "Deep Learning - The Straight Dope"