# inlcude FLRWSolver in ET

Issue #2549 new
Roland Haas created an issue

FLRWSolver provides cosmological initial conditions based on a Friedmann-Lemaitre-Robertson-Walker (FLRW) spacetime, with and without small perturbations.

It also contains a set of tools to create and analyze the data.

If can be found at: https://github.com/hayleyjm/FLRWSolver_public

1. reporter

Reviewer: @helvi witek

Hi - I’m planning to use FLRWSolver in my research (I’m a PhD student at Queen Mary University of London working on testing GR/modified gravity in cosmology), so having FLRWSolver included in the ET would be really useful to me

Hi, I am Michele, a PhD student at the Center for Theoretical Physics in Warsaw and ET user. I believe that having the FLRWSolver thorn included in the ET would be extremely useful for all the users interested in cosmology. The thorn has already a place among the main codes for general-relativistic simulations of cosmological structure formation (see arXiv:2003.08014). I've used the FLRWSolver in my research on relativistic light propagation and observables extrapolation, see arXiv:2107.06306, and I plan to use the thorn in my future works.

Hi, I am a postdoc at the Max Planck Institute for gravitational Physics in Hannover. Before this I did my PhD on inhomogeneous cosmology and the impact of structure formation in the late Universe on the large-scale dynamics, and this is still part of my research interests. As such, I am currently using the ET and the FLRWSolver thorn for some simple cosmological setups and planning to use them in the near future for more involved cosmological simulations. I think that it would be very useful for such applications of the ET to have it include FLRWSolver in future releases.

I’m a PhD student from Argentina and I’m also interested in using FLRWSolver for future applications. I think it would be very useful if it’s included in the ETK.

I’m a PhD student at the University of Canterbury, New Zealand, working with Hayley Macpherson (the maintainer of the thorn) and simulations using FLRWSolver are a major component of my PhD research. Having FLRWSolver included in the ET would be useful for me and others doing research in numerical relativistic cosmology.

2. reporter

A thornlist stanza to check out FLRWSolver can look like this:

# FLRWSolver
# strange path is due to repo name
!TARGET   = $ARR !TYPE = git !URL = https://github.com/hayleyjm/FLRWSolver_public.git !NAME = FLRWSolver !REPO_PATH= ../$2
!CHECKOUT =
EinsteinInitialData/FLRWSolver


where I mimic the naming convention (up to capitialization) suggested in README.Compilation to name the checkout FLRWSolver rather than FLRWSolver_public (this also makes the stanza using a bit less strange REPO_PATH).

Currently (without running bulder.py) fails to compile so is not “harmless” to include in master (since it prevents the thornlist from compiling).

3. reporter

Trying to run builder.py I find that it requires the cffi package which should be mentioned (as well as any other dependencies not in the base Python set of packages).

4. reporter
5. reporter

Has there been any progress on this?

6. reporter

Has there been any progress on this?

7. reporter

With the release out in November it is now time again to start looking into this seriously. Hayley do you need any help bringing the code up to readiness? Helvi would you like an additional reviewer to help out with reviewing the code?

8. reporter
9. reporter

@Zach Etienne assigning to you as the new reviewer.

10. reporter

Pierre Mourier may be able to help with review.

11. reporter

Pierre is not available due to time constraints, same for Michele. Hayley and Roland looking for more reviewer candidates urgently.

12. reporter

Review has started, Zach is in contact with Hayley, who did some code cleanup.

13. reporter

Chi Tian agreed to provide science level input if needed.

Issue 1: Initial data thorns should depend on *Base thorns, and not any particular evolution thorn. I’m referring to

shares:GRHydro

USES real rho_abs_min
USES real rho_rel_min
USES REAL initial_rho_abs_min
USES REAL initial_rho_rel_min
USES REAL initial_atmosphere_factor
USES real GRHydro_rho_central


within param.ccl . It should be the initial data thorn’s responsibility to set up the atmosphere and set its own atmosphere parameters accordingly. Then it’s up to the user to set evolution thorn parameters such that this atmosphere is either maintained or replaced with a different one. For me, the solver wouldn’t even get to the compile stage because I generally comment out GRHydro from my ThornList (naturally I use IllinoisGRMHD).

Oh, I also checked and confirmed that the only other thorn with shares: GRHydro is GRHydro_InitData, which seems appropriate.

Issue 2: Compiler warning needs to be resolved (looks scary):

src/powerspec_ics.F90:16: Warning: While tracing include dependencies: Include file "fftw3.f03" not found

Issue 3: The test/ directory contains a parameter file and sample output data, which has no problem running and seems to use little resources. Great work! What’s missing is the required test.ccl file that sets acceptable tolerances when running this thorn through our unit testing infrastructure. In fact I’m pretty sure that test.ccl must exist here for the directory to even be seen by our unit testing infrastructure.

Issue 4: doc/cactus.sty should be removed. I understand that this is a convenient location for the file when running e.g., pdflatex documentation.tex from within the doc directory, but this file already exists in the Toolkit in a standard place that is looked at when e.g., make AllDoc is run. It's best that this remains the situation in case we'd like to update the style file.

Issue 5: I’m a bit confused as to why check_metric() is something that we do at the end of each initial data routine. All it does is:

    if (CCTK_EQUALS (metric_type, "physical")) then
! do nothing
else
call CCTK_WARN (0, "Unknown value of ADMBase::metric_type -- FLRW only set-up for metric_type = physical")
endif


This parameter cannot change while FLRWSolver is being run, or at all after the parfile is read in, so it only needs to be checked once. In fact I think there’s a scheduling bin for it: ParamCheck.

Comment 1: I really like README.Compilation as it contains some useful “gotchas” that folks often run into when trying to compile the Toolkit more generally. For example:

--> Generally, if you have a nontrivial error with a particular thorn, first try removing that thorn from the thornlist and re-compile.
The code will complain if other thorns on your thornlist depend on the thorn you have just removed. You can also remove some of these if they are not important.
If it turns out a thorn you really need has a dependency on that thorn, ONLY THEN spend the time debugging.


I feel like the contents of this file would make an excellent contribution to a wiki page.

Issue 6: I couldn’t find reference to the example power spectrum (see powerspectrum/) in the documentation. This would be a useful addition. Also I liked that when I went into powerspectrum/ I was greeted with a README file explaining the basics of what this was.

Comment 2: documentation.tex only refers to one Python script in tools, despite there being 5 scripts in that directory and a Jupyter notebook. It would be so nice to reference these in the documentation, as they are likely to be very useful to new users. Also I really like that there’s a tutorial!

Issue 7: There are two, slightly different, copies of the Plot_ET_data.ipynb tutorial in the repo: one in doc/tutorial and one in tools/. This could make the thorn more difficult to maintain moving forward.

Comment 3: Instead of storing simfactory configs for machines within a thorn’s repo, consider contributing them to SimFactory .

Issue 8: There are two FLRW_singlemode.par files: one in par/ and one in test/. These seem to be two very different files, so adjusting the filenames to be more descriptive would help new users browsing these example parameter files.

Issue 9: Running par/FLRW_powerspectrum.par results in a FORTRAN runtime error, as /path/to/flrwsolver/powerspectra/FLRW_matterpower_synchronous_z1000.dat does not exist. It would be better to check for the file’s existence in the Einstein Toolkit CCTK_*ERROR infrastructure, and would be a great opportunity to tell the user how to generate this file.

Issue 10: A more detailed description of what par/FLRW_powerspectrum_restart.par does in that file would be appreciated. It’s rather unusual and probably violates ET standards to have a separate parameter file just for restarting from checkpoints.

Issue 11: random.F90 contains a code snippet “found on stackexchange”, but does not contain a link to the SE article. Complete attribution needed.

Comment 4: I’m rather confused as to why random.F90 exists. It seems to be a custom RNG. Why not use something off the shelf?

Issue 12: init_tools.F90 contains routines from another library “MESCALINE” but does not contain a link to the library or author attribution. Possible software license violation.

Hi all, I have reviewed the documents and majority of the the codes. At least at physics level, everything looks fine to me. Only some tiny issues regarding the way FLRWSolver dealing with the extrinsic curvature:

1. It seems there is a typo in Eq. (10) in the document, where minus sign is missing (at least for the case where the author mentions “Setting f =1/3 and n=2 in the above yields ∂tα=∂ta for EdS“).
2. The name of the variable kvalue in several flrw_*.f90 files seems a little confusing. From its name, it seems this variable represents the extrinsic curvature K. However, its true value seems to be 3 * kvalue if my understanding was correct.
3. By following the gauge condition \dot{\alpha} = f * K, effectively it is setting the coordinate time to something closed to the conformal time \eta, and the value of \alpha will grow roughly following the growth of the scale factor a. Could this possibly introduce numerical instabilities to GRhydro when starting from a very high redshift, and universe keeps expanding for quit long? Since I would guess most of astrophysical GR simulations may choose gauge conditions that make \alpha close to 1.

14. reporter

I have added FLRWSolver to the main ET thornlist as:

# FLRW cosmological initial data
!TARGET   = $ARR !TYPE = git !URL = https://github.com/hayleyjm/FLRWSolver_public !REPO_PATH = ../$2_public
!CHECKOUT =
FLRWSolver/FLRWSolver


which checks out FLRWSolver_public and creates a thorn FLRWSolver/FLRWSolver for it.

If you would rather use a different arrangement or thorn name, please let me know.

Thanks Roland! I usually put FLRWSolver in EinsteinInitialData/FLRWSolver, but whatever you think works best is fine by me.

15. reporter

Sure. I can move it. Some groups like the “branding” of the arrangement. Any location is fine with me though (well except the Cactus* locations which come with special license requirements). I will update the thornlist.

@Zach Etienne the warning you saw was that:

/data/rhaas/postdoc/gr/cactus/ET_trunk/arrangements/FLRWSolver/FLRWSolver/src/powerspec_ics.F90:16: Warning: While tracing include dependencies: Include file "fftw3.f03" not found
Searched in thorn directory and in [/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/sim/build/FLRWSolver/../FFTW3, /data/rhaas/postdoc/gr/cactus/ET_trunk/configs/sim/build/FLRWSolver/../SpaceMask]
Checking status of thorn IDBrillData


? If so the most likely it can be ignored. It is due to Cactus trying to set up makefile dependencies for include files in Fortran code. Those are maybe non-trivial (and may not correctly take ExternalLibraries into account, being rarely used). If that is the one then it is no showstopper though I will try and take a look (reluctant as I am to change anything in that part of CST, fearing ripple effects if I touch anything…).

@Roland Haas : Yes that was the warning I saw. Probably innocuous but better to be safe than sorry, by looking into it.

@Zach Etienne thanks for your review! I have addressed all of the issues and comments you have raised, and pushed the changes to the repo.
In addition to the changes to address your issues/comments, I have also re-named the source files and routines themselves to be more consistent with ET standards.
Here I’ll explain what I’ve done:

• Issue 1: removed these statements from param.ccl as they were unused anyway
• Issue 2: Roland said he would have a look into this, so I’ll leave this be for now
• Issue 3: added a test.ccl file
• Issue 4: removed doc/cactus.sty
• Issue 5: thanks for this! I’ve made a new routine FLRW_ParamCheck and scheduled it at ParamCheck as you suggested, and removed this code from the ID routines
• Issue 6/Comment 2: I’ve added a comment on the example powerspectrum file in the documentation, and added a new section explaining each of the files in the tools/ directory.
• Issue 7: I removed doc/tutorial/ and moved the README.tutorial file to tools/
• Comment 3: I removed configs/ from the public repo and will consider contributing these to simfactory at some point
• Issue 8: re-named the test par to FLRW_singlemode_test.par since the main differences are the resolution and simplified initial parameters
• Issue 9: thanks for pointing this out. I’ve added a call to CCTK_WARN after checking if the supplied file exists, with a useful error explaining next steps for the user
• Issue 10: removed the restart par file from par/
• Issue 11/Comment 4: thanks for this - I realised random.F90 was a lot more complicated than it needed to be. It already uses the intrinsic fortran PRNG, but had some added complexity in generating a seed if it wasn’t given. In FLRWSolver, there will always be a random seed. I removed random.F90 and replaced it with a new, much simpler routine in FLRW_InitTools which directly calls the Fortran intrinsic PRNG and casts this into random numbers following a normal distribution
• Issue 12: Mescaline is my private post-processing code, so I’ve removed this comment

Thanks again for your help and effort on this! Let me know if there’s anything else or if you have questions about my changes.

1. Thanks for this, you’re right there was a minus sign missing which I’ve now fixed
2. I’ve changed the name of this variable from kvalue → kdiag_bg, since it represents the diagonal value of the extrinsic curvature K_ij for the background (not the trace)
3. Yes, you are correct that this choice of gauge means that for the background we have alpha → a and therefore coordinate time can be more or less interpreted as conformal time. I don’t think this choice of evolving lapse will give numerical errors (which I don’t see, as far as I can tell), because the actual time evolution is dependent on the time step in terms of coordinate time, dt, which is set via the grid spacing and the chosen Courant factor. This time step does not change as the lapse evolves, and I don’t think the lapse will have an impact here. I could be wrong, so @Roland Haas please correct me if so!

Thanks again for your time and for helping with this on such short notice!

16. reporter

Just catching up with things (for half a day and then I will falling behind again), I cannot comment on the actual physics. Certainly the description of the Courant factor is correct. There are certainly bits and pieces in the ET that will make assumptions like “lapse is about 1 at large radii” but those are mostly say gravitational wave extraction which is less useful in this situation (since it also implicitly assumes a Schwarzschild like background for the waves).

Having a large lapse should be fine, if I am not mistaken then eg Zach has a trick to smooth out initial gauge dynamics by temporarily setting a very large lapse even for black hole simulations (though likely only near the black holes). Anyway, the existing codes in the ET should (fingers crossed) all handle 0 < lapse < infty at least in cases where this makes sense.