-
Notifications
You must be signed in to change notification settings - Fork 16
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
Automatically supply inverses of known functions #78
Automatically supply inverses of known functions #78
Conversation
Thanks. I also have similar functionality to this in The code in The reason for not adding the inverses yet is because I was debating if this functionality warrants its own package. Being able to invert a function has applications beyond distributions / forecasting (for example, the |
Ah yeah, I love the idea of allowing others to register inverses. Probably makes sense to do this in another package as you say. Having the expressions rewrite themselves into something human-readable would be great too, rather than my solution of just wrapping functions. Especially if the API (from the registration side) could have reduced boilerplate; e.g. something like Speaking of Re: this PR, what are you thinking? If you don't take the full PR, there are a few minor fixes in here that are probably worth poaching (e.g., a missing |
The boilerplate should be possible, and something that I'd like to have as an option for users also. I also don't mind changing the As for the PR, the fixes definitely should be grabbed. I also don't mind merging the inversion functions but if we can determine the scope of the new package it shouldn't take long to put it together and get it on CRAN. The current scope I have in mind is:
Is there anything you would add to the scope? The I think deferring derivatives to |
Agreed.
I like the idea of the main user-facing interface being a simple function where you provide an expression and it returns the inverted expression (or maybe a function). I think having a representation of the pair of (function, inverse) will also be useful, particularly for some package code --- e.g. being able to convert to/from the (function, inverse) representation from
I think being able to optionally provide the derivatives will be helpful. There are some common cases where |
Perhaps we can extend |
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.
LGTM, thanks!
Will remove the inverse transformation parts when the new package with more general functionality is ready.
cool thanks! |
Hi --- as I've been mocking up more examples and use cases for {distributional} with {ggdist} one of the things that I kept running into was the error
Inverting transformations for distributions is not yet supported
, usually when transforming a distribution and then trying to visualize it. This is because visualizing distributions with ggdist typically requires all ofcdf()
,quantile()
, anddensity()
to be defined, and these do not all work without knowing the inverse transformation.While I figured out I could use
dist_transformed()
directly to work around this (e.g. by replacing things likelog2(dist_XXX())
withdist_transformed(dist_XXX(), log2, function(x) 2^x)
, this seemed awkward, since we have several known elementary functions with inverses handled by theMath
andOps
generics. So, this pull request modifies those generics to automatically supply inverses for several functions.This allows you to visualize pretty arbitrary chains of transformations without manually specifying the inverse. A very silly example (this uses the
unify-dist
branch of ggdist; i.e.install_github("mjskay/ggdist@unify-dist")
):Let me know if this seems reasonable. Happy to take feedback and adjust as needed.