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

max_abs_speed_naive for curvilinear Euler, MHD, etc. #869

Closed
15 tasks done
Tracked by #720
andrewwinters5000 opened this issue Sep 20, 2021 · 8 comments
Closed
15 tasks done
Tracked by #720

max_abs_speed_naive for curvilinear Euler, MHD, etc. #869

andrewwinters5000 opened this issue Sep 20, 2021 · 8 comments
Labels
breaking consistency Make Michael happy

Comments

@andrewwinters5000
Copy link
Member

andrewwinters5000 commented Sep 20, 2021

The topic of including orientation and/or normal information for the wave speed estimates came up in this discussion.

TODO:

@ranocha
Copy link
Member

ranocha commented Sep 20, 2021

@trixi-framework/principal-developers What's you opinion on this?

@ranocha ranocha mentioned this issue Sep 20, 2021
36 tasks
@jlchan
Copy link
Contributor

jlchan commented Sep 20, 2021

I'm in favor of this, and it looks like it's been done for APE already.

If incorporating orientation/normals involves the normal component of velocity, it might be possible to meet minimize how invasive the changes are by dispatching to a 1D wave speed estimate.

@ranocha
Copy link
Member

ranocha commented Sep 21, 2021

Since I was the one asking in the thread linked above, I'm of course also in favor of this.

@andrewwinters5000
Copy link
Member Author

I am also in favor of doing this. In some sense it will make the wave speed estimates still naive, but less so, because we at least account for the geometry of the element.

@ranocha
Copy link
Member

ranocha commented Sep 21, 2021

Consensus in the meeting: Update everything; @andrewwinters5000 starts with shallow water in his PR. Then, check the list at the top.

@andrewwinters5000
Copy link
Member Author

I think that the scaling used in this function for acoustic_perturbation_2d.jl

λ_max = (max(abs(v_ll), abs(v_rr)) + max(c_mean_ll, c_mean_rr)) * norm(normal_direction)

is wrong (similar to the bug fix from #831). I believe it should be

λ_max = max(abs(v_ll), abs(v_rr)) + max(c_mean_ll, c_mean_rr) * norm(normal_direction)

because the normal_direction already is scaled by the norm.

This should be fixed in this issue too.

@sloede
Copy link
Member

sloede commented Sep 23, 2021

Thanks for checking this out! In case you fix it also for the APE, please ping @lchristm for a review as well, such that he is aware of the change.

@andrewwinters5000
Copy link
Member Author

Now that the compressible Euler branch passed tests and merged I think this issue can be closed

@jlchan jlchan closed this as completed Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking consistency Make Michael happy
Projects
None yet
Development

No branches or pull requests

4 participants