Skip to content

Add a type guard for intX (#4569) #6319

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
wants to merge 2 commits into from

Conversation

michaelosthege
Copy link
Member

These are the changes from #4569 cherry-picked from 3447619 on the v3 branch.

Closes #4279

Checklist

Major / Breaking Changes

  • None

Bugfixes / New features

  • Integer dtypes are now not downcasted in intX

Docs / Maintenance

  • None

* add type guard for inX
* fix test for pandas
* fix posterior test, ints passed for float data

Closes pymc-devs#4279
@michaelosthege michaelosthege added this to the v5.0.0 milestone Nov 20, 2022
@michaelosthege michaelosthege self-assigned this Nov 20, 2022
@codecov
Copy link

codecov bot commented Nov 20, 2022

Codecov Report

Merging #6319 (7c86ed7) into main (c4a79ef) will decrease coverage by 11.83%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #6319       +/-   ##
===========================================
- Coverage   94.24%   82.41%   -11.84%     
===========================================
  Files         111      111               
  Lines       23930    23936        +6     
===========================================
- Hits        22553    19727     -2826     
- Misses       1377     4209     +2832     
Impacted Files Coverage Δ
pymc/aesaraf.py 87.34% <100.00%> (-7.28%) ⬇️
pymc/distributions/discrete.py 99.23% <100.00%> (+<0.01%) ⬆️
pymc/tests/distributions/util.py 90.86% <100.00%> (ø)
pymc/tests/backends/test_base.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/backends/test_ndarray.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/distributions/test_logprob.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/distributions/test_shape_utils.py 0.00% <0.00%> (-99.77%) ⬇️
pymc/tests/distributions/test_timeseries.py 0.00% <0.00%> (-99.52%) ⬇️
pymc/tests/distributions/test_mixture.py 0.00% <0.00%> (-99.36%) ⬇️
pymc/tests/step_methods/hmc/test_quadpotential.py 0.00% <0.00%> (-95.82%) ⬇️
... and 15 more

@ricardoV94
Copy link
Member

The original issue was about not downcasting int64 to int32, I think it's fine to still apply the old rule in we don't have either of those

@michaelosthege
Copy link
Member Author

The original issue was about not downcasting int64 to int32, I think it's fine to still apply the old rule in we don't have either of those

I'm not sure what you mean. Should we proceed here, or close this and #4279 was wontfix?

@ferrine
Copy link
Member

ferrine commented Nov 21, 2022

This issue was raised because of some discrete likelihoods were overflowed in float32 mode essentially due to int downcasting. The typeguard prevented unintentional downcast if input is already int

@ricardoV94
Copy link
Member

ricardoV94 commented Nov 21, 2022

Basically we want to prevent downcasting but not upcasting which the original fix also prevented. If you still allow upcasting you won't see the issues you were trying to fix in the new commit

@ricardoV94 ricardoV94 self-assigned this Dec 6, 2022
@ricardoV94 ricardoV94 removed this from the v5.0.0 milestone Dec 7, 2022
@michaelosthege
Copy link
Member Author

stale

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

Successfully merging this pull request may close these issues.

Silent integer overflow in test_values
3 participants