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

Bugfix/234 #262

Merged
merged 4 commits into from
Nov 11, 2024
Merged

Bugfix/234 #262

merged 4 commits into from
Nov 11, 2024

Conversation

SeppeStaelens
Copy link
Contributor

This PR fixes the evolution of the gauge variable B in the IntegratedMovingPuncture gauge by adding a new function to both gauge classes that adds matter terms to the evolution equations of the gauge variables.

KAClough and others added 3 commits October 18, 2024 09:55
…ons/peter-evans/create-pull-request-7

Bump peter-evans/create-pull-request from 6 to 7
@SeppeStaelens SeppeStaelens added the bug Something isn't working label Nov 4, 2024
@SeppeStaelens SeppeStaelens linked an issue Nov 4, 2024 that may be closed by this pull request
@KAClough
Copy link
Member

KAClough commented Nov 5, 2024

This looks great and it compiles and runs fine. Sorry to ask for more, but could we change the Kerr example to use the Integrated puncture gauge, just to give an example of how it works to use a different gauge and have a test that this now runs ok? It is actually the better gauge for Kerr because it is not initially conformally flat, which is where you want the integrated version.

@SeppeStaelens
Copy link
Contributor Author

Hi Katy, sure I can adapt the Kerr example. Do you mean also keep the change to the IMP gauge in the main repo? And with a test, do you mean just doing a run, checking the results, or actually implementing a test in the continuous integration (i.e. the checks GitHub runs)?

@KAClough
Copy link
Member

KAClough commented Nov 5, 2024

Yes, sorry! I meant let's update the Kerr example in the main repo so that it uses the IMP gauge. At the moment both vacuum examples use MP which is fine but it is nice to demonstrate that both gauges can work. In terms of a test, just seeing that it runs sensibly, the gauge stabilises and doesn't blow up should be good enough! I guess we also need to set B^I equal to the initial \barGamma^i. It would be nice to have a matter example with IMP too, but we can do that in the future.

@SeppeStaelens
Copy link
Contributor Author

SeppeStaelens commented Nov 5, 2024

Ah yes, I see what you mean. I will change the gauge in the Kerr example, and have it run - I will let you know!

Copy link
Member

@KAClough KAClough left a comment

Choose a reason for hiding this comment

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

This all looks good and compiles and runs ok.

@KAClough KAClough merged commit 3e6efab into main Nov 11, 2024
47 checks passed
@KAClough KAClough deleted the bugfix/234 branch November 11, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with Gauge implementation
2 participants