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

Raise SamplingError if model doesn't have free RVs #4955

Merged
merged 1 commit into from
Aug 21, 2021
Merged

Raise SamplingError if model doesn't have free RVs #4955

merged 1 commit into from
Aug 21, 2021

Conversation

stestoll
Copy link
Contributor

This small PR changes sample() such that it checks for free RVs in the model immediately (before checking start point), and raises a SamplingError instead of a ValueError if the model doesn't have any free RVs.

I think raising a SamplingError is better than a ValueError, since nothing value-wise is wrong with the model itself, just sampling is not possible. Also, SamplingError is more specific than ValueError.

@codecov
Copy link

codecov bot commented Aug 19, 2021

Codecov Report

Merging #4955 (09d5d73) into main (4b57c2f) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4955      +/-   ##
==========================================
- Coverage   74.08%   74.06%   -0.03%     
==========================================
  Files          86       86              
  Lines       13862    13862              
==========================================
- Hits        10270    10267       -3     
- Misses       3592     3595       +3     
Impacted Files Coverage Δ
pymc3/sampling.py 85.67% <100.00%> (ø)
pymc3/backends/report.py 89.51% <0.00%> (-2.10%) ⬇️

Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The message is also easier to understand.

@canyon289
Copy link
Member

LGTM

@canyon289 canyon289 merged commit 7312736 into pymc-devs:main Aug 21, 2021
@stestoll stestoll deleted the emptymodelsamplingerror branch August 21, 2021 15:51
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.

3 participants