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

Remove CholeskyGrad Op #1302

Merged
merged 1 commit into from
Nov 24, 2022
Merged

Conversation

sudarsan2k5
Copy link
Contributor

Fixes #1098

This PR removes CholeskyGrad Op and all the tests related to it.

Here are a few important guidelines and requirements to check before your PR can be merged:

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • pre-commit is installed and set up.
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR.
  • There are tests covering the changes introduced in the PR.

@brandonwillard brandonwillard added the refactor This issue involves refactoring label Nov 17, 2022
brandonwillard
brandonwillard previously approved these changes Nov 20, 2022
@brandonwillard
Copy link
Member

We have to squash these before/when we merge; otherwise, this looks good, thanks!

@codecov
Copy link

codecov bot commented Nov 20, 2022

Codecov Report

Merging #1302 (bdb521b) into main (14c394d) will increase coverage by 0.05%.
The diff coverage is n/a.

❗ Current head bdb521b differs from pull request most recent head aed5f93. Consider uploading reports for the commit aed5f93 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1302      +/-   ##
==========================================
+ Coverage   74.10%   74.15%   +0.05%     
==========================================
  Files         174      174              
  Lines       48673    48629      -44     
  Branches    10373    10364       -9     
==========================================
- Hits        36067    36061       -6     
+ Misses      10315    10277      -38     
  Partials     2291     2291              
Impacted Files Coverage Δ
aesara/tensor/slinalg.py 95.40% <ø> (+11.00%) ⬆️

@rlouf
Copy link
Member

rlouf commented Nov 24, 2022

I just rebased your changes on main, will merge if the tests pass. Thank you for contributing!

@rlouf rlouf merged commit c3b3270 into aesara-devs:main Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor This issue involves refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove CholeskyGrad Op
3 participants