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

Support preliz distributions? #61

Open
aloctavodia opened this issue Jul 5, 2024 · 4 comments
Open

Support preliz distributions? #61

aloctavodia opened this issue Jul 5, 2024 · 4 comments

Comments

@aloctavodia
Copy link
Contributor

PreliZ distributions are similar to scipy ones in many aspects, but sometimes it is more convenient to work with them.

Some differences are that both continues and discrete PreliZ's distributions has methods pdf and logpdf (no pmf or logpmf). And some methods like logcdf or isf are missing.

@OriolAbril
Copy link
Member

Looked a bit into this today, I think the most relevant difference when it comes to the wrappers here is that distribution parameters can only be passed at initialization time in preliz, as opposed to either init time or method calling time as in scipy. The wrappers store the parameters passed at init time but they don't pass them to the distribution when initializing. Instead, when calling a method, the base parameters stored are broadcasted with the arguments and all are passed to the method.

Not sure if it would be easier to try and update the existing classes to initialize the class when calling a method using the broadcasted parameters, then call the method of the initialized distribution with the rest of the arguments or to create a different class for preliz that reuses part of the machinery but handles this part differently.

@aloctavodia
Copy link
Contributor Author

If you want to directly modify a frozen PreliZ distribution you can do this.

dist = pz.Normal(0, 1)
dist._parametrization(0, 2)

@OriolAbril
Copy link
Member

I recently learned about the ongoing (started quite recently) refactor of scipy distributions, which are similar to Preliz ones. Consequently, I think it will be worth modifying the logic of our wrappers from current design:

class RV:
    def method_xyz(*args, **kwargs):
        # like in scipy, here args and kwargs can be both for the method itself or for
        # initializing the distribution
        broadcasted_inputs = broadcast(*args, **kwargs, self.args_provided_at_init)
        return wrapped_distribution_instance.method_xyz(broadcasted_input)

to

class RV:
    def __init__(distribution_class, *args, **kwargs):
        # store distribution and args, kwargs, so far the same
        # no initialization of distribution/conversion to instance

    def method_xyz(*args, **kwargs):
        broadcasted_inputs = broadcast(*args, **kwargs, self.stored_args_kwargs)
        dist_args, method_args = split_arguments(broadcasted_inputs)
        dist_instance = self.distribution_class(dist_args)
        return dist_instance.method_xyz(method_args)

@OriolAbril
Copy link
Member

Potential side effect is the split_arguments function might need to know which distribution it is doing it for

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

No branches or pull requests

2 participants