Skip to content

Propagate static shape in MaxAndArgmax #206

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

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

covertg
Copy link
Contributor

@covertg covertg commented Feb 3, 2023

Motivation for these changes

MaxAndArgmax propagates broadcastable flags but not static shape information. Now we have full shape info. Connected to #203

>>> x = pt.max_and_argmax([[1,2,3],[4,5,6]],axis=1)
>>> x[0].type.shape, x[1].type.shape 
((2), (2,))

Implementation details

I've updated the test suite to test this in a number of places. I'd appreciate close eyes on this to make sure I did the tests right.

I also parametrized a few of the updated tests to help me identify a few cases that were failing. Currently I've left them that way since I figure that less monolithic tests are preferable.

Checklist

Major / Breaking Changes

na

New features

as stated

Bugfixes

na

Documentation

  • ...

Maintenance

  • ...

@codecov-commenter
Copy link

Codecov Report

Merging #206 (871177b) into main (a6e7722) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #206   +/-   ##
=======================================
  Coverage   80.06%   80.06%           
=======================================
  Files         170      170           
  Lines       45185    45185           
  Branches     9608     9608           
=======================================
  Hits        36176    36176           
  Misses       6794     6794           
  Partials     2215     2215           
Impacted Files Coverage Δ
pytensor/tensor/math.py 90.69% <100.00%> (ø)

@michaelosthege
Copy link
Member

LGTM, thanks @covertg !

@ricardoV94 @ferrine does one of you want to take a look and merge?

@michaelosthege michaelosthege added enhancement New feature or request shape inference labels Feb 5, 2023
Copy link
Member

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

LGTM

@ricardoV94 ricardoV94 merged commit 53cad9b into pymc-devs:main Feb 6, 2023
@covertg covertg mentioned this pull request Feb 6, 2023
4 tasks
@ricardoV94 ricardoV94 changed the title Propagate static shape in MaxAndArgmax Propagate static shape in MaxAndArgmax Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request shape inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants