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.

Sign up for GitHub

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

Jump to bottom

Remove intX and floatX from distributions #7114

Merged
merged 18 commits into from
Feb 17, 2024
Merged

Remove intX and floatX from distributions #7114

merged 18 commits into from
Feb 17, 2024

Conversation

aerubanov
Copy link
Contributor

@aerubanov aerubanov commented Jan 23, 2024 •
edited by ricardoV94
Loading

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7114.org.readthedocs.build/en/7114/

@aerubanov aerubanov marked this pull request as draft January 23, 2024 15:02
@codecov Codecov
Copy link

codecov bot commented Jan 23, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (627a8dd) 90.41% compared to head (00ca006) 92.35%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7114      +/-   ##
==========================================
+ Coverage   90.41%   92.35%   +1.93%     
==========================================
  Files         101      101              
  Lines       16930    16922       -8     
==========================================
+ Hits        15308    15629     +321     
+ Misses       1622     1293     -329
Files Coverage Δ
pymc/distributions/bound.py 100.00% <100.00%> (ø)
pymc/distributions/continuous.py 97.53% <100.00%> (-0.01%) ⬇️
pymc/distributions/discrete.py 99.10% <100.00%> (-0.01%) ⬇️
pymc/distributions/mixture.py 94.95% <100.00%> (-0.03%) ⬇️
pymc/distributions/multivariate.py 93.78% <100.00%> (-0.02%) ⬇️
pymc/distributions/simulator.py 84.17% <100.00%> (-0.12%) ⬇️
pymc/distributions/timeseries.py 94.45% <100.00%> (ø)
pymc/distributions/transforms.py 98.47% <100.00%> (-0.02%) ⬇️

... and 6 files with indirect coverage changes

@aerubanov
Copy link
Contributor Author

aerubanov commented Jan 29, 2024 •
edited
Loading

Also, tests/variational/test_inference.py::test_fit_start[SVGD-mini] fails but I do not sure is it related with changes in this PR. This test just skipped when using pytest==7.* and fails with pytest==8.0. I think it`s related with exceptions and warning handling order inside pytest.

@aerubanov aerubanov marked this pull request as ready for review January 29, 2024 14:58
@aerubanov aerubanov changed the title remove floatX() from distribution/continuous.py remove floatX() from distributions Jan 29, 2024
@aerubanov
Copy link
Contributor Author

@ricardoV94 could you please take a look?

@ricardoV94 ricardoV94 changed the title remove floatX() from distributions Remove floatX() from distributions Feb 5, 2024
@ricardoV94 ricardoV94 changed the title Remove floatX() from distributions Remove floatX from distributions Feb 5, 2024
This was referenced Feb 5, 2024
Closed
Closed
ricardoV94
ricardoV94 reviewed Feb 6, 2024
View reviewed changes
tests/distributions/test_truncated.py Outdated
@@ -419,6 +419,7 @@ def test_truncated_gamma():
np.testing.assert_allclose(
logp_pymc,
logp_scipy,
rtol=2.1e-07,
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? We shouldn't have changed results numerically

@ricardoV94 ricardoV94 mentioned this pull request Feb 6, 2024
Closed
@ricardoV94 ricardoV94 changed the title Remove floatX from distributions Remove intX and floatX from distributions Feb 6, 2024
This was referenced Feb 6, 2024
Closed
Closed
aerubanov and others added 5 commits February 9, 2024 15:34
@aerubanov
fix test in TestInitvalAssignment
58635f3
@ricardoV94 @aerubanov
Fix check in CAR make_node to also work with SparseConstants
f564f46
@aerubanov @ricardoV94
Update pymc/distributions/multivariate.py
9e77a0f 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@aerubanov @ricardoV94
Update tests/distributions/test_mixture.py
7837795 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@aerubanov @ricardoV94
Update pymc/distributions/multivariate.py
f2b5ef5 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@aerubanov aerubanov force-pushed the remove-floatx-calls-from-distribution branch from d78216a to f2b5ef5 Compare February 9, 2024 12:34
@aerubanov
Copy link
Contributor Author

@ricardoV94 I added some changes, could you please take a look?

@aerubanov
Copy link
Contributor Author

@ricardoV94 just friendly reminder about PR)

@ricardoV94 ricardoV94 merged commit 3a304d6 into pymc-devs:main Feb 17, 2024
23 checks passed
@ricardoV94
Copy link
Member

Thanks @aerubanov !

@aerubanov aerubanov deleted the remove-floatx-calls-from-distribution branch February 19, 2024 08:02
This was referenced Feb 23, 2024
Closed
Closed
@ricardoV94 ricardoV94 added the major Include in major changes release notes section label Mar 15, 2024
This was referenced May 22, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardoV94 ricardoV94 ricardoV94 approved these changes

Assignees
No one assigned
Labels
maintenance major Include in major changes release notes section
Projects
None yet
Milestone
No milestone
2 participants
@aerubanov @ricardoV94