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

Add softmax to math #5279

Merged
merged 2 commits into from
Jan 20, 2022
Merged

Add softmax to math #5279

merged 2 commits into from
Jan 20, 2022

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Dec 22, 2021

This PR adds softmax and log_softmax wrappers to the new Aesara Ops which accept the axis argument. The wrappers are there just to suppress the deprecation warnings issue on the Aesara side.

Closes #4226

@codecov
Copy link

codecov bot commented Dec 22, 2021

Codecov Report

Merging #5279 (8af7d80) into main (333f7f3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5279   +/-   ##
=======================================
  Coverage   80.40%   80.41%           
=======================================
  Files          82       82           
  Lines       14126    14132    +6     
=======================================
+ Hits        11358    11364    +6     
  Misses       2768     2768           
Impacted Files Coverage Δ
pymc/math.py 69.90% <100.00%> (+1.21%) ⬆️
pymc/step_methods/metropolis.py 82.97% <100.00%> (-0.08%) ⬇️

@ricardoV94
Copy link
Member Author

ricardoV94 commented Dec 23, 2021

Some scary looking errors on windows, most likely triggered by the new Aesara version: https://github.com/pymc-devs/pymc/runs/4605536352?check_suite_focus=true#step:7:21

aesara-devs/aesara#701 would be my first guess

@twiecki twiecki changed the title Add softmax math Upgrade Aesara and add softmax math Dec 23, 2021
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.

This PR is blocked by aesara-devs/aesara#707 and will be delayed until we can upgrade to an Aesara release that includes a fix.

@michaelosthege michaelosthege marked this pull request as draft January 13, 2022 14:38
@ricardoV94 ricardoV94 changed the title Upgrade Aesara and add softmax math Add softmax to math Jan 20, 2022
@ricardoV94 ricardoV94 marked this pull request as ready for review January 20, 2022 12:35
def softmax(x, axis=None):
# Ignore vector case UserWarning issued by Aesara. This can be removed once Aesara
# drops that warning
with warnings.catch_warnings():
Copy link
Member Author

Choose a reason for hiding this comment

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

I checked locally that this does not screw up UserWarnings elsewhere

@twiecki twiecki merged commit 9af9320 into pymc-devs:main Jan 20, 2022
@ricardoV94 ricardoV94 deleted the add_softmax_math branch January 20, 2022 18:44
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.

Suggestion: Add tt.nnet.softmax to pm.math?
3 participants