AMReX-Astro / MAESTROeX

A C++ low Mach number stellar hydrodynamics code
https://amrex-astro.github.io/MAESTROeX/
BSD 3-Clause "New" or "Revised" License
40 stars 22 forks source link

base state long vs. int #426

Closed zingale closed 5 months ago

zingale commented 5 months ago

In MaestroMakeEdgeState.cpp, we do:

          ParallelFor(nr_fine, [=] AMREX_GPU_DEVICE(long r) {                                                                                                                                                                
              Real slope = 0.0;                                                                                                                                                                                              
              ...                                                                                                                                                                                                                            

but we have a lot of implicit narrowing to int here. An issue is that we use these ints to index into the base state arrays:

/home/zingale/development/MAESTROeX/Exec/science/urca/../../../Source/MaestroMakeEdgeState.cpp:73:25: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions]
   73 |                 int g = r + ng;
      |                         ^
/home/zingale/development/MAESTROeX/Exec/science/urca/../../../Source/MaestroMakeEdgeState.cpp:86:29: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions]
   86 |                     int g = r + ng - 1 + i;
      |                             ^
/home/zingale/development/MAESTROeX/Exec/science/urca/../../../Source/MaestroMakeEdgeState.cpp:112:39: warning: narrowing conversion from 'long' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions]
  112 |             Real u = 0.5 * (w0_arr(0, r) + w0_arr(0, r + 1));
      |                                       ^

How do we deal with this? Do we really need the long there in the loop? If so, do we need to change the base state arrays to take a long as an index?

biboyd commented 5 months ago

Looked through and all the r's are either indexing BaseStateArray's or are being used to calculate an int like g = r + ng;.

I think it would be best to just change the loops from long to int.

Further down (line 442) there are more loops that could similarly be changed. Here j is only used to compute int r = j + lo;

                ParallelFor(hi - lo + 1, [=] AMREX_GPU_DEVICE(long j) {
                    Real slope = 0.0;
                    int r = j + lo;
zingale commented 5 months ago

okay, do you want to PR this?

biboyd commented 5 months ago

Yes, I can get a PR going