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

Do not always remap storage in fgraph_to_python #1291

Merged

Conversation

brandonwillard
Copy link
Member

@brandonwillard brandonwillard commented Nov 10, 2022

This PR fixes an issue resulting from re-mapped storage cells in fgraph_to_python. The issue was discovered in #1289 via TestOpFromGraph.test_grad.

Basic OpFromGraph support for Numba is added in this PR as well.

  • Write more direct/explicit tests

@brandonwillard brandonwillard added bug Something isn't working Numba Involves Numba transpilation labels Nov 10, 2022
@brandonwillard brandonwillard marked this pull request as draft November 10, 2022 20:22
@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Merging #1291 (d56381e) into main (1390cc3) will increase coverage by 0.01%.
The diff coverage is 97.29%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1291      +/-   ##
==========================================
+ Coverage   74.10%   74.12%   +0.01%     
==========================================
  Files         174      174              
  Lines       48624    48654      +30     
  Branches    10351    10353       +2     
==========================================
+ Hits        36035    36064      +29     
+ Misses      10301    10300       -1     
- Partials     2288     2290       +2     
Impacted Files Coverage Δ
aesara/link/utils.py 60.18% <90.90%> (-1.67%) ⬇️
aesara/link/numba/dispatch/basic.py 92.68% <100.00%> (+0.21%) ⬆️
aesara/link/numba/dispatch/scalar.py 87.41% <100.00%> (+0.08%) ⬆️
aesara/tensor/random/basic.py 99.03% <100.00%> (+0.02%) ⬆️
aesara/link/jax/dispatch/extra_ops.py 95.52% <0.00%> (+8.95%) ⬆️

@brandonwillard brandonwillard changed the title Do not reuse outer-graph storage in Numba conversions of Composites Do not remap outer-graph storage in fgraph_to_python Nov 10, 2022
@brandonwillard brandonwillard changed the title Do not remap outer-graph storage in fgraph_to_python Do not always remap storage in fgraph_to_python Nov 10, 2022
@brandonwillard brandonwillard marked this pull request as ready for review November 11, 2022 00:54
@brandonwillard brandonwillard merged commit 31b77a2 into aesara-devs:main Nov 11, 2022
@brandonwillard brandonwillard deleted the fix-Numba-composite-storage branch November 11, 2022 05:27
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant