athena icon indicating copy to clipboard operation
athena copied to clipboard

Missing documentation and instructions for creating user-defined Coordinates class; Get/Compute naming convention is confusing

Open felker opened this issue 1 year ago • 2 comments

Brought up by Lizhong Zhang at the 2nd Athena++ Workshop. This page on the Wiki: https://github.com/PrincetonUniversity/athena/wiki/Coordinate-Systems-and-Meshes should be expanded with another section (although it is already quite long) about the code modifications that are necessary for adding a new (relativistic and/or nonrelativistic?) Coordinates derived class to Athena++. Should probably include:

  • Adding a new .cpp file to coordinates/ directory
  • Modifying coordinates.hpp
  • Need to modify:https://github.com/PrincetonUniversity/athena/blob/99abbff15962419d773ee3f830c03cc8f6a8d914/configure.py#L335 if the new coordinate system is not GR/SR
  • Virtual functions that should be overridden; class arrays that should be filled.
  • What needs to be precomputed and stored, and what functions are just for convenience.
  • Why we decided not to precompute and store a bunch of quantities like length, area, volume in Coordinates class arrays?

Anything else?


Also, related:

Our Coordinates abstract base class has 3x types of methods that are "Get"-prefixed counterparts to existing methods:

Real GetEdgeXLength(const int k, const int j, const int i);
...
Real GetFaceXArea(const int k, const int j, const int i);
...
Real GetCellVolume(const int k, const int j, const int i);

The non-"Get" functions fill a provided AthenaArray at a given k,j along an x1 slice. These functions imply that they are "getters" for accessing private members/arrays of the Coordinates class, but they actually compute the quantities like their counterparts, just at a single cell.

A user/developer may have no idea why there are seemingly 2x versions of the same type of function (including me, I completely forgot):

  • The GetEdgeXLength() functions seem to only be used in mesh refinement flux corrections?

  • Same for the GetFaceXLength() functions, but they are also used in the GR Torus problem generator.

  • GetCellVolume() is only used in a few problem generators

  • [ ] Do we rename the Get functions?

felker avatar May 16 '23 22:05 felker

Just to clarify the relativistic/nonrelativistic issue for anyone reading this thread: This task is about writing new Coordinates classes deep inside the code, modifying src/coordinates/ in the process. Such coordinates might be used for any physics.

Orthogonal to this, the GRUser Coordinates class already exists and is documented, allowing those using GR to specify arbitrary metrics by enrolling simple functions in their pgen files. Thus GR users don't need to write new classes, which makes sense, since the whole essence of GR is to abstract away from any particular instantiations of coordinates.

c-white avatar May 17 '23 00:05 c-white

Naming conventions aside, I updated the wiki with some description of how to add new coordinates.

c-white avatar Jan 02 '24 19:01 c-white