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 solve_triangular output when overwrite_b=True #1235

Merged

Conversation

jessegrabowski
Copy link
Member

@jessegrabowski jessegrabowski commented Feb 23, 2025

Description

Issue #1233 found a bug in the numba mode of solve_triangular. Basically, we can't get around copying the incoming matrices if they're not already F-contiguous. Also adds a regression test for this bug.

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pytensor--1235.org.readthedocs.build/en/1235/

@jessegrabowski jessegrabowski added bug Something isn't working numba linalg Linear algebra labels Feb 23, 2025
Copy link

codecov bot commented Feb 23, 2025

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 81.99%. Comparing base (5d4e9e0) to head (16d09b9).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pytensor/link/numba/dispatch/slinalg.py 0.00% 5 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1235      +/-   ##
==========================================
- Coverage   81.99%   81.99%   -0.01%     
==========================================
  Files         188      188              
  Lines       48566    48567       +1     
  Branches     8677     8677              
==========================================
  Hits        39823    39823              
- Misses       6580     6581       +1     
  Partials     2163     2163              
Files with missing lines Coverage Δ
pytensor/link/numba/dispatch/slinalg.py 44.52% <0.00%> (-0.09%) ⬇️

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.

Just some questions and a small suggestion

@jessegrabowski jessegrabowski force-pushed the fix-solve-tri-overwrite-b branch from 3f23a5d to 16d09b9 Compare February 24, 2025 03:36
@jessegrabowski jessegrabowski merged commit e7dec4d into pymc-devs:main Feb 24, 2025
72 of 73 checks passed
@jessegrabowski jessegrabowski deleted the fix-solve-tri-overwrite-b branch February 24, 2025 04:51
Aarsh-Wankar pushed a commit to Aarsh-Wankar/pytensor that referenced this pull request Feb 24, 2025
* Fix bug in solve_triangular when `overwrite_b = True`

* Add regression test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linalg Linear algebra numba
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Numba solve_triangular returns wrong results when overwrite_b=True
2 participants