-
Notifications
You must be signed in to change notification settings - Fork 114
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
Generalize Jin Xin relaxation #1961
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
f_ll_base = get_block_components(u_ll, 2, equations) | ||
f_rr_base = get_block_components(u_rr, 2, equations) | ||
dissipation = SVector((sqrt_velocities .* (u_rr_base - u_ll_base))..., | ||
(sqrt_velocities .* (f_rr_base + f_ll_base))..., |
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.
@ranocha Are you sure this is supposed to be a +
and not a -
, as it was in @gregorgassner's original implementation?
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.
Should be -
- sorry, my fault 🙈
f_rr_base = get_block_components(u_rr, 3, equations) | ||
dissipation = SVector((sqrt_velocities .* (u_rr_base - u_ll_base))..., | ||
zero(u_ll_base)..., | ||
(sqrt_velocities .* (f_rr_base + f_ll_base))...) |
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.
Same here
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.
My fault
return max(sa[1],sa[2],sa[3],sa[4]), max(sb[1],sb[2],sb[3],sb[4]) | ||
@inline function max_abs_speeds(u, equations::JinXinEquations{2}) | ||
return ntuple(Val(ndims(equations))) do n | ||
maximum(equations.sqrt_velocities_inv[n]) |
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.
maximum(equations.sqrt_velocities_inv[n]) | |
maximum(equations.sqrt_velocities[n]) |
Again, was this by accident or do you really think it should be the inverse of the sqrt of the velocities?
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.
No, should be without inv
- looks like I hacked this stuff together too quickly
I decided to merge this as it is now but for now I reverted the behavior to what it was before. |
I drafted a generalized version of Jin Xin relaxation that should also work for other equations - at least the stuff in the
equations
directory, not the implicit stuff. How does that look to you, @sloede?