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

Simplify logic with variadic_add and variadic_mul helpers #932

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Jul 15, 2024

Just a small cleanup. The need to know whether we are adding / multiplying 0, 1, or more inputs logic shows up in several places (also in our downstream packages) that I think we're better of just adding a helper

This change revealed a bug in the grad of TrueDiv that was also fixed, and I cleaned up the insane test that revealed this error (the error would still be found with the new test)

@ricardoV94 ricardoV94 removed the request for review from aseyboldt October 7, 2024 11:48
@ricardoV94 ricardoV94 force-pushed the add_mul_variadic branch 2 times, most recently from d6d55cc to 2e545cd Compare October 7, 2024 13:08
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 7 lines in your changes missing coverage. Please review.

Project coverage is 81.75%. Comparing base (1c2bc8f) to head (86e4283).
Report is 108 commits behind head on main.

Files with missing lines Patch % Lines
pytensor/scalar/basic.py 53.84% 6 Missing ⚠️
pytensor/tensor/blas.py 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #932      +/-   ##
==========================================
+ Coverage   81.73%   81.75%   +0.01%     
==========================================
  Files         183      183              
  Lines       47734    47708      -26     
  Branches    11611    11589      -22     
==========================================
- Hits        39016    39003      -13     
+ Misses       6523     6514       -9     
+ Partials     2195     2191       -4     
Files with missing lines Coverage Δ
pytensor/ifelse.py 51.70% <100.00%> (ø)
pytensor/tensor/basic.py 91.54% <100.00%> (ø)
pytensor/tensor/math.py 91.13% <100.00%> (-0.16%) ⬇️
pytensor/tensor/rewriting/basic.py 94.11% <100.00%> (-0.04%) ⬇️
pytensor/tensor/rewriting/blas.py 88.16% <100.00%> (-0.05%) ⬇️
pytensor/tensor/rewriting/math.py 89.81% <100.00%> (+0.39%) ⬆️
pytensor/tensor/rewriting/subtensor.py 90.32% <100.00%> (+0.20%) ⬆️
pytensor/tensor/subtensor.py 89.32% <100.00%> (ø)
pytensor/tensor/blas.py 65.54% <50.00%> (+0.17%) ⬆️
pytensor/scalar/basic.py 80.41% <53.84%> (ø)

... and 6 files with indirect coverage changes

@ricardoV94 ricardoV94 requested a review from Armavica October 8, 2024 11:13
@ricardoV94 ricardoV94 merged commit 5632777 into pymc-devs:main Oct 8, 2024
60 of 61 checks passed
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.

2 participants