Cleanup UniformCartesian #753

forrestglines · 2022-10-06T23:51:58Z

PR Summary

Cleanup the current UniformCartesian coordinate class to have fewer functions, utilizing templates on coordinate direction to reduce the number of functions. Done in preparation to implement other coordinate system classes.

Here are the new functions and any differences highlighted

Position of elements (previouslyx1v, x1f, and x1s2*)

Xc<int dir>(const int idx); //Cell Centers
Xf<int face, int dir>(const int idx); //Face Centers
Xe<int edge, int dir>(const int idx); //Edge Centers
Xn< int dir>(const int idx); //Cell Nodes

where 1<=dir<=3 is which component of the direction being queried (i.e. x,y,z), face and edge specify which face/edge is being queried, and idx is the index along dir. The addition of face and edge along with the functions Xe and Xn are new. These allow for consistently getting the positions of faces, edges, and nodes with one function without mixing calls to Xc and Xf to get the position of off-cell-center locations.

*In particular with Xf, when face==dir it returns the same as x1f etc. did in the old version. Now when face!=dir, it returns the face-averaged position of the other directions, replacing x1s2, x2s3, etc.

I've also dropped versions using i,j,k since the uniform cartesian, spherical, and cylindrical coordinates do not need them. If we want non-uniform coordinate systems later, we'll have to go back and change them.

Distance between elements (previously dx1v, dx1f etc. )

Dxc<int dir>(const int idx); //Distance between cell centers along dir
Dxf<int dir, int face>(const int idx); //Distance between face centers along dir
Dxe<int dir, int edge>(const int idx); //Distance between edge centers along dir
Dxn< int dir>(const int idx); //Distance between cell Nodes along dir

Likewise, face, edge, Dxe, and Dxn are new to get distances between different elements along different dimensions.

Integration elements (previously Dx, Area, Edgelength, and Volume)

CellWidth<int dir>( const int k, const int j, const int i); //Cell width at cell center
FaceArea<int face>( const int k, const int j, const int i); //Area of Face
EdgeLength<int edge>( const int k, const int j, const int i); //Length of edge
CellVolume( const int k, const int j, const int i); //Cell Volume

These functions take all three indices to accommodate extension to spherical and cylindrical. They need k,j, and i for the sake of spherical and cylindrical coordinates but for now these arguments are unenforced in Parthenon.

~~Area-averaged positions on face-centers (previously x1s2, x3s1 etc. )~~

Xs<int dir, int face>( int idx);

These are used in the AMR machinery for face-centered fields, which is currently missing from Parthenon. However, I believe these are identical to Xf<int dir, int face> for the uniform coordinates proposed. Xs might not be necessary anymore.
*I removed Xs since the new Xf can give all the same information unambiguously.

For each of these functions templated on dir, face, edge, etc, we can introduce undercase version with runtime parameters, i.e.

xc( const int dir, const int idx); //Cell Centers
faceArea( const int dir, const int k, const int j, const int i); //Face Areas

This convention avoids naming conflicts between simplified functions and runtime functions. We can implement these later as needed.

We'll probably need to implement more coordinate functions as we need them for cylindrical and spherical coordinates.

Also, here is a script to do renaming of coordinates in downstream codes:

#!/bin/bash

#Rename x\1v( -> Xc<\1>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)v(/.Xc<\1>(/g" {} \;

#Rename x\1f( -> Xf<\1>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)f(/.Xf<\1>(/g" {} \;

#Rename x\1e( -> Xe<\1>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)e(/.Xe<\1>(/g" {} \;

#Rename x\1s\2( -> Xs<\1,\2>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)s\([123]\)(/.Xs<\1,\2>(/g" {} \;

#Rename dx\1v -> Dxc<\1>)
find ./ -name "*.*pp" -exec sed -i "s/\.dx\([123]\)v(/.Dxc<\1>(/g" {} \;

#Rename dx\1f -> Dxf<\1>
find ./ -name "*.*pp" -exec sed -i "s/\.dx\([123]\)f(/.Dxf<\1>(/g" {} \;

#Rename Dx(, -> CellWidthFA(,
find ./ -name "*.*pp" -exec sed -i "s/\.Dx(/.CellWidthFA(/g" {} \;

#Rename Area(, -> FaceAreaFA(,
find ./ -name "*.*pp" -exec sed -i "s/\.Area(/.FaceAreaFA(/g" {} \;

#Rename Volume(, -> CellVolmue(,
find ./ -name "*.*pp" -exec sed -i "s/\.Volume(/.CellVolume(/g" {} \;

PR Checklist

Code passes cpplint
New features are documented.
Adds a test for any bugs fixed. Adds tests for new features.
Code is formatted
Changes are summarized in CHANGELOG.md
CI has been triggered on Darwin for performance regression tests.
(@lanl.gov employees) Update copyright on changed files

forrestglines · 2022-10-26T18:56:12Z

I ended up implementing a slightly different interface for UniformCartesian between my last comment and what's currently implemented/inherited from Athena++. Here are the new functions and any differences highlighted

Position of elements (previouslyx1v, x1f, and x1s2*)

Xc<int dir>(const int idx); //Cell Centers
Xf<int face, int dir>(const int idx); //Face Centers
Xe<int edge, int dir>(const int idx); //Edge Centers
Xn< int dir>(const int idx); //Cell Nodes

where 1<=dir<=3 is which component of the direction being queried (i.e. x,y,z), face and edge specify which face/edge is being queried, and idx is the index along dir. The addition of face and edge along with the functions Xe and Xn are new. These allow for consistently getting the positions of faces, edges, and nodes with one function without mixing calls to Xc and Xf to get the position of off-cell-center locations.

*In particular with Xf, when face==dir it returns the same as x1f etc. did in the old version. Now when face!=dir, it returns the face-averaged position of the other directions, replacing x1s2, x2s3, etc.

I've also dropped versions using i,j,k since the uniform cartesian, spherical, and cylindrical coordinates do not need them. If we want non-uniform coordinate systems later, we'll have to go back and change them.

Distance between elements (previously dx1v, dx1f etc. )

Dxc<int dir>(const int idx); //Distance between cell centers along dir
Dxf<int dir, int face>(const int idx); //Distance between face centers along dir
Dxe<int dir, int edge>(const int idx); //Distance between edge centers along dir
Dxn< int dir>(const int idx); //Distance between cell Nodes along dir

Likewise, face, edge, Dxe, and Dxn are new to get distances between different elements along different dimensions.

Integration elements (previously Dx, Area, Edgelength, and Volume)

CellWidth<int dir>( const int k, const int j, const int i); //Cell width at cell center
FaceArea<int face>( const int k, const int j, const int i); //Area of Face
EdgeLength<int edge>( const int k, const int j, const int i); //Length of edge
Cell Volume( const int k, const int j, const int i); //Cell Volume

These functions take all three indices to accommodate extension to spherical and cylindrical. They need k,j, and i for the sake of spherical and cylindrical coordinates but for now these arguments are unenforced in Parthenon.

~~Area-averaged positions on face-centers (previously x1s2, x3s1 etc. )~~

Xs<int dir, int face>( int idx);

These are used in the AMR machinery for face-centered fields, which is currently missing from Parthenon. However, I believe these are identical to Xf<int dir, int face> for the uniform coordinates proposed. Xs might not be necessary anymore.
*I removed Xs since the new Xf can give all the same information unambiguously.

For each of these functions templated on dir, face, edge, etc, we can introduce undercase version with runtime parameters, i.e.

xc( const int dir, const int idx); //Cell Centers
faceArea( const int dir, const int k, const int j, const int i); //Face Areas

This convention avoids naming conflicts between simplified functions and runtime functions. We can implement these later as needed.

We'll probably need to implement more coordinate functions as we need them for cylindrical and spherical coordinates.

forrestglines · 2022-10-26T18:56:25Z

@par-hermes format

Yurlungur · 2022-10-27T14:59:51Z

I ended up implementing a slightly different interface for UniformCartesian between my last comment and what's currently implemented/inherited from Athena++. Here are the new functions and any differences highlighted

I think these choices make sense. My one concern is regarding Da and Ds but I can't think of better names

forrestglines · 2022-11-01T20:41:28Z

I encountered an error that I'm a little confused about as I tried out the new coordinates in AthenaPK. It occurred in this templated member function that was kind-of pointlessly templated on a coordinate class.

template <typename View1D, typename Coords>
PRhoProfile<View1D>
generate_P_rho_profile(parthenon::IndexRange ib, parthenon::IndexRange jb,
                     parthenon::IndexRange kb, Coords coords) const;

And when I call this function in the class

template <int dir, class ... args>
  KOKKOS_FORCEINLINE_FUNCTION Real Dxc(Args... args) const {
    assert(dir > 0 && dir < 4);
    return dx_[dir - 1];
  }

It causes this error with clang:

/vast/home/forrestglines/code/athenapk-project/athenapk/src/pgen/cluster/hydrostatic_equilibrium_sphere.cpp:176:28: error: reference to non-static member function must be called
  PARTHENON_REQUIRE(coords.Dxc<1>(0) == coords.Dxc<1>(1), "No equidistant grid in x1dir");
                    ~~~~~~~^~~
/vast/home/forrestglines/code/athenapk-project/athenapk/external/parthenon/src/utils/error_checking.hpp:36:9: note: expanded from macro 'PARTHENON_REQUIRE'
  if (!(condition)) {                                                                    \
        ^~~~~~~~~

and this error with CUDA/11.7.0:

/vast/home/forrestglines/code/athenapk-project/athenapk/src/pgen/cluster/hydrostatic_equilibrium_sphere.cpp:176:22: error: invalid operands of types ‘<unresolved overloaded function type>’ and ‘int’ to binary ‘operator<’
   PARTHENON_REQUIRE(coords.Dxc<1>(0) == coords.Dxc<1>(1), "No equidistant grid in x1dir");
           ~~~~~~~~~~~~~~^~~~

I don't think it's related to the PARTHENON_REQUIRE macro since commenting out those lines lead to the same error. I also tried removing the varidic template on Dxc but that had no effect. I got around the issue by removing the templating on Coords in AthenaPK so this is non-blocking for this PR. Does anyone have an idea what might have caused this error though?

Problematic AthenaPK code here:
hydrostatic_equilibrium_sphere.hpp

hydrostatic_equilibrium_sphere.cpp

Work-around AthenaPK code here:
hydrostatic_equilibrium_sphere.hpp

hydrostatic_equilibrium_sphere.cpp

forrestglines · 2022-11-01T20:46:45Z

I used this script to update AthenaPK to use the new coordinate names. It should work for any other derivative Parthenon code (or any Parthenon PR as well).

#!/bin/bash

#Rename x\1v( -> Xc<\1>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)v(/.Xc<\1>(/g" {} \;

#Rename x\1f( -> Xf<\1>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)f(/.Xf<\1>(/g" {} \;

#Rename x\1e( -> Xe<\1>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)e(/.Xe<\1>(/g" {} \;

#Rename x\1s\2( -> Xs<\1,\2>(
find ./ -name "*.*pp" -exec sed -i "s/\.x\([123]\)s\([123]\)(/.Xs<\1,\2>(/g" {} \;

#Rename dx\1v -> Dxc<\1>)
find ./ -name "*.*pp" -exec sed -i "s/\.dx\([123]\)v(/.Dxc<\1>(/g" {} \;

#Rename dx\1f -> Dxf<\1>
find ./ -name "*.*pp" -exec sed -i "s/\.dx\([123]\)f(/.Dxf<\1>(/g" {} \;

#Rename Dx(, -> CellWidthFA(,
find ./ -name "*.*pp" -exec sed -i "s/\.Dx(/.CellWidthFA(/g" {} \;

#Rename Area(, -> FaceAreaFA(,
find ./ -name "*.*pp" -exec sed -i "s/\.Area(/.FaceAreaFA(/g" {} \;

#Rename Volume(, -> CellVolmue(,
find ./ -name "*.*pp" -exec sed -i "s/\.Volume(/.CellVolume(/g" {} \;

Edited once more for changes to API

Yurlungur · 2022-11-01T22:39:20Z

I encountered an error that I'm a little confused about as I tried out the new coordinates in AthenaPK.

That's bizarre. I have no idea.

forrestglines · 2022-11-01T22:43:27Z

I think these choices make sense. My one concern is regarding Da and Ds but I can't think of better names

I renamed these to Dx, Area, EdgeLength, and Volume. I'm thinking about being verbose in the final version to use CellWidth, FaceArea, EdgeLength, and CellVolume. I'd like to hear people's input on this but I'm leaning towards those names for the final edit since they're closer to Athena++'s names.

Yurlungur · 2022-11-01T22:56:12Z

I like being verbose in this case. 👍

pgrete · 2022-11-03T14:28:42Z

👍 for the new naming scheme. I'm also in favor of the more verbose description and cleaned up interface!

bprather

LGTM but confused about template vs runtime call use in one spot, see comment

bprather · 2022-11-10T18:17:45Z

src/mesh/mesh_refinement.cpp

+          const Real area00 = coords.faceArea(X1DIR, k, j, i);
+          const Real area01 = coords.faceArea(X1DIR, k, j + 1, i);
+          const Real area10 = coords.faceArea(X1DIR, k + 1, j, i);
+          const Real area11 = coords.faceArea(X1DIR, k + 1, j + 1, i);


Can this & below be FaceArea<XNDIR>()? If not, is that something we can fix with constexpr?

Similarly, is it worth standardizing all N->XNDIR using this PR, maybe with additional XNFACE variables?

Can this & below be FaceArea<XNDIR>()? If not, is that something we can fix with constexpr?

These can be replaced, I just pushed that change along with a few other files.

Similarly, is it worth standardizing all N->XNDIR using this PR, maybe with additional XNFACE variables?

Maybe? It would mean a lot more function name bloat (i.e. x1f->Xf<X1DIR,X1DIR>) when it should be clear from context that the integers refer to directions and it wouldn't add any extra checks/code safety.

Agreed it's verbose and unwieldy to use the macros. I just kept forgetting what Xf<1, 1> was referring to. Especially in terms of the argument order, since the convention switches by variable. Xf<X1FACE, X1DIR> or Dxf<X1DIR, X1FACE> is at least totally explicit. (I absolutely mixed up both those orders the first time).

It also wouldn't add any type checks, true. Which would make it hard to enforce, since using the macros would be purely a matter of convention and require much more typing.

...but that's given me a better (worse) idea: maybe we should type-check face locations vs edge locations vs directions? I mean, Xf<FaceLocation f, Direction d> seems like even more overkill, but at least then we get the compile-time check benefits and it's easy to enforce a readable standard.

Maybe the answer for now is just to use the Xf<int> shortcut everywhere? That's unambiguous, resembles the old Athena convention, and it's all we need for UniformCartesian, right? And, it and leaves room for a future typed solution, since changing the full macro only involves changing the shortcut and not a bunch of calls scattered around the code.

The Xf<int,int> template is so that we can use Xf for all references to locations, replacing the x*f and x*s* functions in Athena++.

That said, we're currently only using Xf<x,x> in Parthenon (and Athena++) although that will change soonish when we add the edge-corrections for face centered fields on AMR blocks. We can add an overloaded template Xf<int> when both the face and coordinate direction are the same.

For enforcing typing, we would have to change CoordinateDirection from an enum to enum class I believe. Hopefully that wouldn't break anything. Anywhere we really need to covert from an int to this enum would be slightly more awkward. I would be fine with that change as long as people are onboard with the code bloat (in terms of characters added) that would create.

I dunno if I want to move CoordinateDirection to an enum class... that's going to change everything everywhere.

I definitely get why the <int, int> template is necessary. I was proposing something like:

template <int dirface, class... Args> KOKKOS_FORCEINLINE_FUNCTION Real Dxf(Args... args) const { return Dxf<dirface, dirface>(args); }

i.e., just overloading and calling through. I misread the diff and thought that a similar overload was already implemented. I think this would be simple enough to do in a general way, even if the specific code above doesn't work, but the difference is obviously minimal. If this isn't uniformly agreed to be more readable it's not worth it.

Re: making directions (and faces) an enum class, I am abstractly very in favor of this. Probably not in this PR though; it was just an idea for someday, and as Jonah says it will mean a lot of lines changed, even if there's always a clear way to translate the code.

template <int dirface, class... Args> KOKKOS_FORCEINLINE_FUNCTION Real Dxf(Args... args) const { return Dxf<dirface, dirface>(args); }

I see, that should be implemented, I've now added it in.

I dunno if I want to move CoordinateDirection to an enum class... that's going to change everything everywhere.

That's mostly why I'm hesitant to make that change. But with just the enum, we don't have any type checking, so using the enum everywhere at the moment won't add much. In the few places where Xf<int,int> will be used, it could be advantageous to use different enums to differentiate between coordinate direction and face direction.

Re: making directions (and faces) an enum class, I am abstractly very in favor of this. Probably not in this PR though; it was just an idea for someday, and as Jonah says it will mean a lot of lines changed, even if there's always a clear way to translate the code.

Yep, let's save it for another PR

Yurlungur

Overall I like the new naming scheme.

I was able to go through Phoebus and build and run with the new uniform-cartesian setup but it was not as trivial as I'd hoped. A number of locations in the code required manual intervention. I list a few questions/comments related to my manual interventions below.

IMO none of these are blocking, but I'd like to chat about them before we merge, so I'm requesting changes for now, and if you push back about the changes, I'll just click approve. :)

src/coordinates/uniform_cartesian.hpp

Yurlungur · 2022-11-18T19:35:06Z

src/coordinates/uniform_cartesian.hpp

+  template <int dir, int face, class... Args>
+  KOKKOS_FORCEINLINE_FUNCTION Real Dxf(Args... args) const {
+    assert(dir > 0 && dir < 4 && face > 0 && face < 4);
+    return dx_[face - 1];
+  }


I'm not sure why... but this function in particular seemed to throw C++'s type deduction through a loop. A number of places, where I had

const auto &coords = pmb->coords;

I had to change to

const parthenon::Coordinates_t &coords = pmb->coords;

for template resolution of, e.g.,

coords.DXf<1,1>(i)

to resolve properly.

Hmm, is this class... Args causing the type deduction causing issues? Did it happen on multiple compilers?

I'm not sure... that's the likeliest culprit but I didn't see what was wrong. Only tried on gnu.

I don't think this is urgent to fix, by the way.

This issue came up when I switched from function argument functions to template arguments in the Poisson example.

/vast/home/forrestglines/code/parthenon-project/parthenon/example/poisson/poisson_package.cpp(233): error: expected an expression /vast/home/forrestglines/code/parthenon-project/parthenon/example/poisson/poisson_package.cpp(162): error: expected an expression 2 errors detected in the compilation of "/vast/home/forrestglines/code/parthenon-project/parthenon/example/poisson/poisson_package.cpp".

Changing out auto fixed the compilation issue

Yurlungur · 2022-11-18T19:44:01Z

I also notice a number of tests seem to be failing... possibly related to the template type deduction issue I noticed above.

forrestglines · 2022-12-01T20:09:01Z

I also notice a number of tests seem to be failing... possibly related to the template type deduction issue I noticed above.

What tests where are failing? In Parthenon or in Phoebus? The last merge introduced code using the old naming conventions so I need to fix those before it will compile.

Yurlungur · 2022-12-01T20:13:50Z

I just meant tests in parthenon. Phoebus tests passed after I fixed things.

forrestglines · 2022-12-08T04:22:04Z

@jonahm-LANL This is ready for review again

Yurlungur

Sorry for the delay---been under the weather. LGTM. Two minor nitpicks below. Neither blocking.

Yurlungur · 2022-12-13T02:19:45Z

src/interface/swarm_device_context.hpp

-            std::floor((x - x_min_) / coords_.Dx(CoordinateDirection::X1DIR))) +
+            std::floor((x - x_min_) / coords_.CellWidthFA(CoordinateDirection::X1DIR))) +
        ib_s_;
-    j = (ndim_ > 1) ? static_cast<int>(std::floor(
-                          (y - y_min_) / coords_.Dx(CoordinateDirection::X2DIR))) +
-                          jb_s_
-                    : jb_s_;
-    k = (ndim_ > 2) ? static_cast<int>(std::floor(
-                          (z - z_min_) / coords_.Dx(CoordinateDirection::X3DIR))) +
-                          kb_s_
-                    : kb_s_;
+    j = (ndim_ > 1)
+            ? static_cast<int>(std::floor(
+                  (y - y_min_) / coords_.CellWidthFA(CoordinateDirection::X2DIR))) +
+                  jb_s_
+            : jb_s_;
+    k = (ndim_ > 2)
+            ? static_cast<int>(std::floor(
+                  (z - z_min_) / coords_.CellWidthFA(CoordinateDirection::X3DIR))) +
+                  kb_s_
+            : kb_s_;


I've noticed lots of places where you've made this change: Dx -> CellWidthFA. I think the better replacement rule would be Dx -> Dxc<dir> as that's probably the intent in all these places and it'll be the faster option down the road. For examples it also offers more concise and parthenonic code.

If I recall correctly, Dx was a run time determined function inherited from Athena++ and thus became CellWidthFA from the Athena++ comments on its behavior vs. dx1v. I think it might make a difference for some coordinate systems. Everywhere Dx was used in Parthenon, however, would be better served by Dxc<dir> so I'll make that change

Yurlungur · 2022-12-13T02:20:27Z

src/outputs/vtk.cpp

@@ -146,10 +146,10 @@ void VTKOutput::WriteContainer(SimTime &tm, Mesh *pm, ParameterInput *pin, bool
    // write x1-coordinates as binary float in big endian order
    std::fprintf(pfile, "X_COORDINATES %d float\n", ncoord1);
    if (ncells1 == 1) {
-      data[0] = static_cast<float>(pmb->coords.x1v(out_ib.s));
+      data[0] = static_cast<float>(pmb->coords.Xc<1>(out_ib.s));


Does the vtk output even work? Not for this MR but maybe we should either revive it for real or kill it.

Yurlungur · 2022-12-20T01:30:50Z

@forrestglines can you share the most recent version of your script to find/replace all the instances?

pgrete · 2023-01-02T13:28:57Z

The updated script in the comment above did the job for parthenon-hydro

forrestglines · 2023-01-06T22:33:19Z

The script in the comment should work. I forgot that I had an old script in the description of the PR. I've now updated that to match the new interface

Yurlungur · 2023-01-07T00:24:05Z

Cool thanks, @forrestglines @pgrete

This comment was marked as outdated.

Sign in to view

forrestglines added 6 commits October 25, 2022 15:29

Rename dx*v->Dxc

895f9f9

Rename dx*f -> Dxf

7bdcc56

Rename x*v -> Xc<*>, x1f -> Xf<*,*>

0ff0b11

Rename x*s* -> Xs<face,dir>

42d3d78

Rename Area->da, Volume->dv, Dx->dx

a25f0b9

Fixed bug in Xf, Xc

72c6002

forrestglines force-pushed the forrestglines/cleanup_uniform_cartesian branch from cb93802 to 72c6002 Compare October 26, 2022 00:17

forrestglines added 3 commits October 26, 2022 10:11

Reverse dir and face to match order for old x1s2

1cf4e79

Rename dv->Dv

32d9b1f

Fixes for Dxf, Dxc

7f62762

forrestglines added 3 commits October 27, 2022 10:38

Formatting

5819f45

Merge branch 'develop' into forrestglines/cleanup_uniform_cartesian

8434fed

Rename da->area, Dv->Volume

2e4781d

Add asserts on face-dir

e7262af

forrestglines added 3 commits November 1, 2022 14:49

Merge branch 'develop' into HEAD

055eddf

Fix to asserts

d0e2c9c

Add coordinates documentation

bdf4cd2

forrestglines changed the title ~~WIP:Cleanup UniformCartesian~~ Cleanup UniformCartesian Nov 1, 2022

forrestglines added 2 commits November 1, 2022 17:22

Rename Dx->CellWidth, Area->FaceArea, Volume->CellVolume

dc74026

Updated docs

c8a44a1

bprather self-requested a review November 10, 2022 18:09

bprather approved these changes Nov 10, 2022

View reviewed changes

Changed from run to compile time functions

44f4452

forrestglines requested review from jdolence, pgrete, Yurlungur and lroberts36 November 15, 2022 18:56

Merge branch 'develop' into forrestglines/cleanup_uniform_cartesian

7427c79

Yurlungur requested changes Nov 18, 2022

View reviewed changes

Merge branch 'develop' into forrestglines/cleanup_uniform_cartesian

a3a874b

forrestglines and others added 5 commits December 1, 2022 18:12

Formatting

3c9bbbe

Added (k,j,i) operators, Dxf<d>

b388837

Merge branch 'develop' into forrestglines/cleanup_uniform_cartesian

6f4ac0e

Rename cellWidth->CellWidthFA

d9769ce

Merge branch 'develop' into forrestglines/cleanup_uniform_cartesian

0d0c145

Yurlungur approved these changes Dec 13, 2022

View reviewed changes

Changed CellWidthFA->Dxc<> as needed

234a61a

forrestglines enabled auto-merge December 14, 2022 17:54

forrestglines merged commit db78ad8 into develop Dec 14, 2022

Yurlungur deleted the forrestglines/cleanup_uniform_cartesian branch December 15, 2022 17:31

pgrete mentioned this pull request Jan 2, 2023

Bump Parthenon 0.8.0 (roughly) and Kokkos 3.7.01 parthenon-hpc-lab/parthenon-hydro#4

Merged

Cleanup UniformCartesian #753

Cleanup UniformCartesian #753

Conversation

forrestglines commented Oct 6, 2022 • edited Loading

PR Summary

PR Checklist

This comment was marked as outdated.

This comment was marked as outdated.

forrestglines commented Oct 26, 2022 • edited Loading

forrestglines commented Oct 26, 2022

Yurlungur commented Oct 27, 2022

forrestglines commented Nov 1, 2022

forrestglines commented Nov 1, 2022 • edited Loading

Yurlungur commented Nov 1, 2022

forrestglines commented Nov 1, 2022

Yurlungur commented Nov 1, 2022

pgrete commented Nov 3, 2022

bprather left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bprather Nov 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bprather Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yurlungur left a comment

Choose a reason for hiding this comment

Yurlungur Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yurlungur commented Nov 18, 2022

forrestglines commented Dec 1, 2022

Yurlungur commented Dec 1, 2022

forrestglines commented Dec 8, 2022

Yurlungur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yurlungur commented Dec 20, 2022

pgrete commented Jan 2, 2023

forrestglines commented Jan 6, 2023

Yurlungur commented Jan 7, 2023

forrestglines commented Oct 6, 2022 •

edited

Loading

forrestglines commented Oct 26, 2022 •

edited

Loading

forrestglines commented Nov 1, 2022 •

edited

Loading

bprather Nov 15, 2022 •

edited

Loading

bprather Dec 1, 2022 •

edited

Loading

Yurlungur Nov 18, 2022 •

edited

Loading