Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sympy #219

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Sympy #219

wants to merge 8 commits into from

Conversation

guyko81
Copy link

@guyko81 guyko81 commented Jan 8, 2021

Adding new distribution: Gamma
Plus providing 2 working examples with Sympy:

  • a simple one, where the distribution, gradients and Fisher information matrix can be calculated in closed form (Normal distribution)
  • a complicated one, where the Fisher information is not closed (metric is not implemented) and the gradients use non-numpy functions (polygamma => need to import separately and need to use the string version of the lambdify function)

This way the development can speed up significantly and a wider community can contribute.

@ryan-wolbeck
Copy link
Collaborator

@guyko81 in order to get the build to function properly you'll have to modify https://github.com/stanfordmlgroup/ngboost/blob/master/pyproject.toml instead of the requirements.txt

@guyko81
Copy link
Author

guyko81 commented Jan 8, 2021

@ryan-wolbeck I can't figure out what's the deal with the failed checks. Nothing on Google that would help.
make: *** [lint] Error 1 Makefile:12: recipe for target 'lint' failed
And why 2 of the commits have a red X next to them? Test fail is noted there?

What I can tell is that the code runs nicely, and as far as I can tell the found models are the same as before.

@alejandroschuler
Copy link
Collaborator

Thanks for providing this example! I don't think it quite fits the bill of what I'd like for the design, though. It seems like the developer would need to understand/use sympy in order to implement new distributions. More importantly, they still need to understand the idea of differentiating the score of the distribution w.r.t. the parameters and the difference between the internal and user-facing parametrizations. These are straightforward concepts to folks with a lot of experience and formal education but I expect that to many users it is a stretch. That's why the current work on the jax branch has it set up so that the user doesn't even need to implement most of the methods in order to get a working prototype.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants