IAS-Astrophysics / athenak

Performance-portable version of the Athena++ astrophysical AMR-MHD code using Kokkos.
BSD 3-Clause "New" or "Revised" License
23 stars 14 forks source link

Port z4c solver into latest available version of master. - [merged] #530

Closed jmstone closed 1 month ago

jmstone commented 1 year ago

In GitLab by @zappa on Apr 13, 2023, 06:21

_Merges z4crebased -> master

Merge request for the z4c solver into master. The changes are as minimal as possible and the kernels of the original code are used for the fundamental parts. The major additions are the finite difference stencils used for discrete derivatives, defined in

src/utils/finite_diff.hpp

and the usage of athena tensor structures, defined from the athena array structures in

src/athena_tensor.hpp

These additions are anyway restricted to the parts of the code concerning the z4c solver.

A detailed (but probably not completely exhaustive) report of the porting of z4c solver in athenak is in porting_report.md

It seems that there are few conflicts that are due to the extension of z4c-specific lines in the following files. It should be fairly easy to merge manually. Conflicts: src/outputs/basetype_output.cpp src/outputs/outputs.hpp src/outputs/restart.cpp src/pgen/pgen.cpp

jmstone commented 1 year ago

In GitLab by @jmstone216 on Apr 13, 2023, 10:39

Excited to see this become a merge request! I noticed the CI pipeline failed on the linting step, likely because the z4c module does not follow the Google/AthenaK style guide. Hengrui and I can decide how to proceed: we'll either have to relax the style rules for the z4c module or fix everything. In addition, perhaps you and Hengrui can fix the conflicts and merge master into this branch, which needs to be done before we can proceed. In the meantime, the reviewers can have a look at the code and send you comments.

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on Apr 20, 2023, 11:00

added 24 commits

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on Apr 20, 2023, 11:17

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 08:43

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 08:47

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 08:51

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 08:53

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 10:13

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 10:54

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 10:58

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 11:02

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 11:07

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 11:15

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 4, 2023, 11:21

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 6, 2023, 15:37

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 6, 2023, 18:30

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 10, 2023, 14:59

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 10, 2023, 15:28

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 10, 2023, 15:33

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 10, 2023, 16:28

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 15, 2023, 09:23

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 15, 2023, 09:34

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 15, 2023, 09:43

Linear wave convergence plots.

Running with wave amplitude 1e-5, I found error stop converging at 1e-8 convergence

To better understand why that stopped converging, I run with different amplitude, 1e-4, 1e-5, 1e-6, and 1e-8. This convergent threshold is some quadratic effect with large coefficient, as shown by the following plot. The error is dominated by the diagonal part of the spatial metric, likely gauge related stuff quadratic

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 15, 2023, 10:15

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 15, 2023, 12:51

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on py/write_FD.py line 1

why no flake8? also consider renaming script write_fd.py

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/adm/adm.cpp line 35

can you change this to pmy_pack->pz4c == nullptr?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/mesh/meshblock.cpp line 118

Are these necessary? On a Cartesian mesh, dx1, dx2, and dx3 are spatially constant within a given MeshBlock, therefore, it wouldn't be too much of a lift to carry out these divisions outside the inner loop.

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/mesh/meshblock_pack.cpp line 175

should we be setting padm = nullptr for runs without z4c (as above, for other physics)?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/outputs/basetype_output.cpp line 122

139 lol. We really need a better design for outputs...

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/outputs/basetype_output.cpp line 491

Can you delete this block?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/pgen/pgen.cpp line 240

Can you delete this block?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/pgen/z4c_two_puncture.cpp line 20

How do you feel about eliminating the two puncture pgen from the main repo since it has external dependencies?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/utils/finite_diff.hpp line 2

Can we eliminate write_fd.py? Also, there are extraneous blank lines and weird indents in this file.

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/z4c/z4c.hpp line 18

same comments as above...

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/z4c/z4c.hpp line 21

Should these live in athena.hpp?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/utils/finite_diff.hpp line 6

I don't understand why we are templating on NGHOST? I think what is really desired here is templating on the spatial order. I could imagine one wanting to operate within a given spatial order independent of what the selection of nghost is...

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/z4c/z4c_adm.cpp line 424

I think you have this all correct, but please go back and check if a barrier is needed here...

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/z4c/z4c_tasks.cpp line 46

same comments as above... Why not introduce a z4c_order param so that the spatial order of the differencing is not tied to the choice for NGHOST?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/z4c/z4c_tasks.cpp line 68

fully comment out?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/z4c/z4c_tasks.cpp line 142

it would be nice to also add rk4 to hydro and mhd...

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/athena_tensor.hpp line 337

Is there more of a Kokkos way to handle this?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/athena_tensor.hpp line 137

is there more of a Kokkos way to handle this?

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on src/athena_tensor.hpp line 258

same as above...

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

Commented on CMakeLists.txt line 124

I vote for eliminating external dependencies in the main repo.

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:19

A huge effort! Congratulations, all! Please find my review comments above.

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:20

Commented on src/outputs/basetype_output.cpp line 491

block starting at 491.

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:21

Commented on src/pgen/pgen.cpp line 240

block starting at 240.

jmstone commented 1 year ago

In GitLab by @pdmullen on May 22, 2023, 11:21

Commented on src/z4c/z4c.hpp line 21

meaning things like POW3, etc.

jmstone commented 1 year ago

In GitLab by @Hengrui_Zhu on May 22, 2023, 16:18

Commented on src/adm/adm.cpp line 35

done!