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

Fix storage and mit-mot handling in numba_funcify_Scan #1203

Merged
merged 7 commits into from
Oct 4, 2022

Conversation

brandonwillard
Copy link
Member

@brandonwillard brandonwillard commented Sep 21, 2022

This PR fixes some storage allocation and access/usage issues in numba_funcify_Scan.

Closes #923

  • Fix truncated Scan input storage support
  • Support basic mit-mots

@brandonwillard brandonwillard self-assigned this Sep 21, 2022
@brandonwillard brandonwillard added bug Something isn't working important Numba Involves Numba transpilation Scan Involves the `Scan` `Op` labels Sep 21, 2022
@brandonwillard brandonwillard force-pushed the refactor-Numba-Scan branch 2 times, most recently from 34d8080 to fd03bd5 Compare September 21, 2022 21:14
@codecov
Copy link

codecov bot commented Sep 21, 2022

Codecov Report

Merging #1203 (aefeeef) into main (69c1044) will increase coverage by 0.00%.
The diff coverage is 97.27%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main    #1203    +/-   ##
========================================
  Coverage   79.13%   79.14%            
========================================
  Files         173      173            
  Lines       48492    48523    +31     
  Branches    10966    10319   -647     
========================================
+ Hits        38374    38403    +29     
- Misses       7625     7628     +3     
+ Partials     2493     2492     -1     
Impacted Files Coverage Δ
aesara/link/numba/dispatch/scan.py 94.11% <96.96%> (-1.44%) ⬇️
aesara/link/numba/dispatch/basic.py 92.46% <100.00%> (+0.01%) ⬆️
aesara/link/numba/dispatch/tensor_basic.py 100.00% <100.00%> (ø)
aesara/link/utils.py 61.25% <100.00%> (+0.12%) ⬆️
aesara/scan/op.py 85.41% <0.00%> (-0.07%) ⬇️
aesara/link/numba/dispatch/scalar.py 87.33% <0.00%> (+1.33%) ⬆️

@brandonwillard
Copy link
Member Author

brandonwillard commented Sep 23, 2022

While working on this, I noticed some inner-function typing issues that—again—involve the distinction between NumPy and Numba scalars (cf. #1063). More specifically, an inner-function could return a Numba scalar for a tap with storage that is typed as a NumPy scalar.

If I can find a reasonable general solution soon, then I'll include it in this PR; otherwise, we can merge this and handle that issue separately.

This should prevent errors when the input is already a Numba scalar, and it
will use Numba's type information to selectively apply the scalar conversion.
Using the `auto_name` values will result in cache misses when caching is based
on the generated source code, so we're not going to use it.
@brandonwillard brandonwillard merged commit 3d96ee8 into aesara-devs:main Oct 4, 2022
@brandonwillard brandonwillard deleted the refactor-Numba-Scan branch October 4, 2022 23:09
@brandonwillard brandonwillard changed the title Fix storage allocation and access in numba_funcify_Scan Fix storage and mit-mot handling in numba_funcify_Scan Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working important Numba Involves Numba transpilation Scan Involves the `Scan` `Op`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Numba Scan fails when sit-sot sequences aren't full length
1 participant