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

Add class to call specificPostTimeStep at t=0 #142

Merged
merged 3 commits into from
Jan 20, 2021

Conversation

TaigoFr
Copy link
Member

@TaigoFr TaigoFr commented Nov 3, 2020

I think many of us suffer from not having our specificPostTimeStep processing also at t=0 (Weyl Extraction or any other extraction you do). This PR comes to fix it. I have done many hacks in the past to fix it, but it's quite tricky because specificPostTimeStep has to be called in reverse order of levels, so it cannot be really called in initialData or smth like that.

This was mostly done by @mirenradia, I just generalized it a bit and above all: try to make it really user friendly.

The idea is basically you can now add just a few lines to Main, and specificPostTimeStep will be called (at t=0). You just have to be careful with defining the first_step to m_time==0. instead of dt as we commonly had before (sometimes by default, so be careful) EXCEPT for the PunctureTracker, that is also called in Main at t=0, and hence a if (m_time != 0) guard saves it inside specificPostTimeStep.

But it really is quite general. You can call any GRAMRLevel function, including custom ones you may define in your own Example (hence why a template Level), and you may choose not to revert the order of the levels. Plus, it's also made to be an AMR Scheduler, mostly due to how I inherited @mirenradia's version, but it's nice that like that you can also choose to use it as a function that is called at regular time intervals (say every 10 timesteps). A bit overkill I agree, but who knows...

The name 'CallFunctionOverLevels' is not amazing, but seemed quite generic. Feel free to offer suggestions if you find better alternatives.

P.S. I also edited MareNostrum's Make.defs.local under to hood.

@TaigoFr TaigoFr force-pushed the feature/call_specificPostTimeStep branch from 618f932 to 239ea0d Compare November 10, 2020 19:11
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.

A few minors changes suggested, I'll take it into my fork for testing.

@TaigoFr TaigoFr force-pushed the feature/call_specificPostTimeStep branch from 239ea0d to 7aedde3 Compare December 13, 2020 20:04
@KAClough
Copy link
Member

KAClough commented Jan 5, 2021

@TaigoFr if you rebase this branch to the master we could merge it next... I am happy to sign off.

One thing I'm a bit undecided about is the comment where you show what to do for scheduling:
// or call at post-plotLevel, at every 'some_interval'
etc

It is very useful info but it feels like this should either be actually used in another example (but I know they don't have a specificPostTimestep yet) or added to the wiki. Maybe you can add it to the wiki page:

https://github.com/GRChombo/GRChombo/wiki/Extraction-and-integration

And then once you update your PR #121 you could implement it there for the Kerr Example?

@TaigoFr TaigoFr force-pushed the feature/call_specificPostTimeStep branch from 7aedde3 to c55d2e1 Compare January 12, 2021 19:22
@TaigoFr
Copy link
Member Author

TaigoFr commented Jan 12, 2021

Rebased.

I hope it's enough to update an example. I can update the Kerr Example of the ADMQuantities PR #121 after this is merged. We can leave a brief mention in the wiki if necessary.

Copy link
Member

@mirenradia mirenradia left a comment

Choose a reason for hiding this comment

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

I have a few very minor changes.

@TaigoFr TaigoFr force-pushed the feature/call_specificPostTimeStep branch from 4057566 to 4499cb7 Compare January 19, 2021 17:29
@mirenradia mirenradia self-requested a review January 20, 2021 17:09
Copy link
Member

@mirenradia mirenradia left a comment

Choose a reason for hiding this comment

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

I am happy for this to be merged now.

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.

Me too!

@KAClough KAClough merged commit 477b302 into master Jan 20, 2021
@KAClough KAClough deleted the feature/call_specificPostTimeStep branch January 20, 2021 17:31
mirenradia pushed a commit to mirenradia/GRChombo-public that referenced this pull request Feb 3, 2023
* Add CallFunctionOverLevels class to call specificPostTimeStep at t=0
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.

3 participants