Skip to content

Conversation

Shreyas-Ekanathan
Copy link
Contributor

No description provided.

C
end

function 🦋2!(C, A::Diagonal, B::Diagonal)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between 🦋2! and 🦋!? Can we unify them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this was one thing I wanted to talk about, currently we have two functions because 🦋! can't accept submatrices generated by view calls, so as of right now we need the second 🦋2! function for those cases, but ideally we would combine those.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the answer here might be the same as to the issue with 🦋! allocating, in that it might be worth allocating a butterfly matrix with the proper diagonals preallocated and fill in the entries manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this wouldn't allow us to pass submatrices into the function right? so we would still need 🦋2!?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could just be a separate method of 🦋!.

end

function 🦋!(C::SparseBandedMatrix, A::Diagonal, B::Diagonal)
setdiagonal!(C, [A.diag; -B.diag], true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[A.diag; -B.diag] allocates. Any way to avoid?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm not sure to be honest

Comment on lines 146 to 147
M = size(U, 1)
Mh = M >>> 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to n and n_half?

@oscardssmith
Copy link

@ChrisRackauckas I think this is good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants