-
Notifications
You must be signed in to change notification settings - Fork 447
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 unused diagonalizers and linear solvers from dfoccwave::Tensor2d and dfoccwave::Array2d #2684
Conversation
@loriab, I'm not comfortable merging this in while the |
Somehow I had the impression this was in occ, so it was fine. I agree hold off for now. |
Sure, no problem. FYI some of the diagonalizer cleanup is blocked until this is in, but I can chip off some other shards. |
PR rebased, CI tests are passing. |
I'd like @loriab's opinion before discussing this one, since there's still some |
Very well. Please let me know if a decision has been reached. This is blocking the last remaining pieces of the diagonalizer rework saga. |
If I'm remembering where I left off and performing the diffs correctly, there's no more changes to be made to those 4 array/tensor files in the dfocc saga. And none of the yet-to-be-PRd code uses
|
Thanks for looking into it. It looks like none of these involve But, some FYI, my plan was to eventually change the function signature of |
This reverts commit 826f09d.
CI is acting up, but otherwise this is ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Negative LOC always good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
Description
Much like
occ
(PR #2679),dfocc
also has these functions that are never called anywhere C++-side, and notPSI_API
.This is another shard of the #2642 mega-PR that can be merged independently.
Todos
Status