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

5-arg mul! for continuously coupled operators? #145

Open
jlchan opened this issue Aug 26, 2022 · 4 comments
Open

5-arg mul! for continuously coupled operators? #145

jlchan opened this issue Aug 26, 2022 · 4 comments

Comments

@jlchan
Copy link
Contributor

jlchan commented Aug 26, 2022

Is there a mul! operation specialized for SummationByPartsOperators.UniformNonperiodicCoupledOperator? If not, I'd be happy to help add it if that's useful.

@ranocha
Copy link
Owner

ranocha commented Aug 29, 2022

I think I should have implemented everything in terms of mul!, e.g.,

function mul!(dest::AbstractVector, cD::UniformCoupledOperator, u::AbstractVector, α=true)
N, _ = size(cD)
@boundscheck begin
@argcheck N == length(u)
@argcheck N == length(dest)
end
@unpack D, meshgrid, coupling, der_order = cD
if coupling === Val(:continuous)
mul!(dest, D, meshgrid, coupling, der_order, u, α)
scale_by_inverse_mass_matrix!(dest, cD)
else
mul!(dest, D, meshgrid, coupling, der_order, u, α)
end
dest
end

If that doesn't work, please let me know! PRs are always welcome, of course.

@jlchan
Copy link
Contributor Author

jlchan commented Aug 29, 2022

Sorry, I didn't realize that was already there. I was looking for methods for UniformNonperiodicCoupledOperator and saw

julia> methods(mul!, (SummationByPartsOperators.UniformNonperiodicCoupledOperator,), SummationByPartsOperators)
# 0 methods for generic function "mul!":

Looking more closely, I think the issue is that I need a 5-argument mul!, which I hope should be easy to add. Would you like me to go ahead and submit a PR for that?

@ranocha
Copy link
Owner

ranocha commented Aug 29, 2022

Ah, I see. Yeah, the case of 5-arg mul! is more complicated for this case. The CGSEM operators are easier to write down at the level of M * D, so I extracted the multiplication by the inverse mass matrix into another function. To get full 5-arg mul!, this needs to be re-written. If you have time to spare, I would love to review a PR from you for this 👍

@jlchan
Copy link
Contributor Author

jlchan commented Aug 29, 2022

Sounds good. I'll try implementing it later this week.

@jlchan jlchan changed the title mul! for continuously coupled operators? 5-arg mul! for continuously coupled operators? Aug 30, 2022
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

No branches or pull requests

2 participants