Skip to content

Require supports_sampler_stats=True for all BaseTrace #6193

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

Closed
michaelosthege opened this issue Oct 9, 2022 · 5 comments
Closed

Require supports_sampler_stats=True for all BaseTrace #6193

michaelosthege opened this issue Oct 9, 2022 · 5 comments

Comments

@michaelosthege
Copy link
Member

We don't have a backend that doesn't support it, and by requiring support we can simplify code.

@williambdean
Copy link
Contributor

I'm interested in taking this issue in order to learn more about the backends submodule.

Could you give a little more details? Is this looking to remove the supports_sampler_stats attribute or just change default to True? I'm see that it is used in a few places.
Is the BaseTrace class still meant to be extended?

Thanks

@michaelosthege
Copy link
Member Author

I think removing it right away should be fine.
My plan is to morph BaseTrace closer to mcbackend.Chain to make it easier to switch.
#6188 was part of that and so is #6192.

I you take on this issue it will help too!

@williambdean
Copy link
Contributor

Great. Thanks for the context @michaelosthege

Okay. What I will do is remove all references to supports_sampler_stats and assume that is True. If there is any logic using supports_sampler_stats = False (ie raising Errors), I will remove those as well unless it depends on another condition.

Basically will follow the flow of logic where supports_sampler_stats = True

I will take a stab and open a PR

@michaelosthege
Copy link
Member Author

sounds good, @wd60622 !
ping me for review when ready

@michaelosthege
Copy link
Member Author

Closed by #6205 / 534b89b

@michaelosthege michaelosthege added the trace-backend Traces and ArviZ stuff label Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants