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

WIP: Sc/polytropic 2d wave speed #1816

Merged
merged 168 commits into from
Feb 20, 2024
Merged

Conversation

SimonCan
Copy link
Contributor

Added wave speeds to the PolytropicEulerEquations2D equations.

SimonCan and others added 30 commits June 26, 2023 16:31
Added interface coupling docs to the main menu.
Moved converter coupling section.
Added some documentation on coupling converters.
@ranocha
Copy link
Member

ranocha commented Jan 31, 2024

Please start from a clean state for future PRs. This one includes a huge amount of unrelated history.

@SimonCan
Copy link
Contributor Author

Restarting this PR from a clean state.

@SimonCan SimonCan closed this Jan 31, 2024
@ranocha
Copy link
Member

ranocha commented Jan 31, 2024

Sorry, that's not necessary for this one. I would just like to ask you to start with a clean state in the future. We can go ahead with this one as it is - whatever you prefer

@SimonCan SimonCan reopened this Feb 1, 2024
@SimonCan
Copy link
Contributor Author

SimonCan commented Feb 5, 2024

We should perform the usual tests for this (consistency & rotation)

Just add the usual rotation and consistency tests. They pass locally.

DanielDoehring
DanielDoehring previously approved these changes Feb 6, 2024
@SimonCan
Copy link
Contributor Author

SimonCan commented Feb 6, 2024

I believe I found a bug in the new wavespeed implementations. I also left a few comments just to clarify how the coupling conversion is done.

The bug should be fixed now.

@SimonCan SimonCan marked this pull request as ready for review February 7, 2024 15:11
@DanielDoehring
Copy link
Contributor

Anything that prevents this from being merged @andrewwinters5000 @SimonCan ?

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

This looks good to me once my (minor) comment is addressed.

src/equations/polytropic_euler_2d.jl Outdated Show resolved Hide resolved
Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

LGTM! We can merge this.

@andrewwinters5000 andrewwinters5000 enabled auto-merge (squash) February 16, 2024 05:03
@andrewwinters5000 andrewwinters5000 merged commit c5e743a into main Feb 20, 2024
33 of 36 checks passed
@andrewwinters5000 andrewwinters5000 deleted the sc/polytropic_2d_wave_speed branch February 20, 2024 18:38
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.

8 participants