E3SM-Project / scream

Fork of E3SM used to develop exascale global atmosphere model written in C++
https://e3sm-project.github.io/scream/
Other
78 stars 56 forks source link

HOMMEXX diagnostic output is not checked in standalone C++-vs-F90 tests #2123

Open ambrad opened 1 year ago

ambrad commented 1 year ago

CmakeVsF90.cmake is intended to compare HOMME's diagnostic output to stdout. However, it does not because its search pattern is not correct, leading to empty strings that are compared. This issue requires:

  1. fixing this problem in homme/cmake/CxxVsF90.cmake.in;
  2. running CIME test HOMME_P24.f19_g16_rx1.A to check for errors that are found from doing the comparison on non-empty data;
  3. if there are diffs in diagnostic output, troubleshooting these.
  4. submitting a PR to upstream.
ambrad commented 1 year ago

Attn: @oksanaguba @bartgol.

oksanaguba commented 1 year ago

i am not sure what was meant by string "diagnostics" there. maybe we at some point had something in the output? right now i see only one way, take this set of lines

 nstep=          12  time=   2.00000000000000       [h]
  u     =  -0.741887479133145E-03 (101)  0.819830421799649E-02 ( 24)  0.123919117996407E+03
  v     =  -0.374131513923748E-01 ( 24)  0.374131513916861E-01 ( 24) -0.634187813375320E-09
  w     =  -0.357413293704489E-03 (  1)  0.209243376142763E-03 (  1)  0.205742428820105E-01
  vTh_dp=   0.234558579365350E+06 (128)  0.114141880372143E+07 (  1)  0.160492366585252E+11
  T     =   0.299785186958484E+03 ( 22)  0.300295993199227E+03 (128)  0.147396469631127E+08
  mu    =   0.999999989921717E+00 (  1)  0.100000000462337E+01 (  1)
  dz(m) =   0.688501700152893E+02 (128)  0.175508001674237E+05 (  1)  0.229680317006746E+08
  dp    =   0.781207535880406E+03 (  1)  0.781273727114283E+03 (  1)  0.384000370899196E+08
    fu  =  -0.319236005650600E-08 (112)  0.116400118270498E-08 (111) -0.729185582213972E-05
    fv  =  -0.268170984118815E-07 (112)  0.268170984141629E-07 (112)  0.190275109578954E-14
    ft  =  -0.289054107579178E-04 ( 29)  0.415683875792044E-04 (128) -0.827085912665128E+00
    fq1 =   0.000000000000000E+00 (  1)  0.288693356806040E-06 (128)  0.739054993423462E-04
      min dz/w (CFL condition) =   0.532287010822666E+07 (104)
 qv(  1)=   0.0000000000000000E+00  0.2660472257512892E-05  0.1741357303453504E-01
    TBOT=   0.299896517388944E+03  0.300295993199227E+03
      ps=   0.999945645926917E+05  0.100003037070628E+06  0.384000370899197E+08
      M =   0.101976718089169E+05 kg/m^2  0.100000001385728E+06mb     
    dry M =   0.101976716676049E+05 kg/m^2
**DYNAMICS**        J/m^2                    W/m^2                W/m^2    
KE,d/dt        0.16569789944436E+01  0.40919219289475E-03
IE,d/dt        0.00000000000000E+00  0.00000000000000E+00
PE,d/dt        0.00000000000000E+00  0.00000000000000E+00
 E,dE/dt       0.16569789944436E+01  0.40919219289475E-03
Q  1,Q diss, dQ^2/dt:  0.14131200313568E-03 kg/m^2  0.3011673E-22 -0.6508691E-17
Change from dribbled phys tendencies, viscosity, remap:
dKE/dt(W/m^2):  -0.4265610E-07 -0.7488353E-06 -0.2724117E-13
dIE/dt(W/m^2):   0.0000000E+00  0.0000000E+00  0.0000000E+00
dPE/dt(W/m^2):   0.0000000E+00  0.0000000E+00  0.0000000E+00
dQ1/dt(kg/sm^2)  0.1962667E-07
(E-E0)/E0   0.800333098427497E+01
(Q-Q0)/Q0   0.100000000000000E+01   Q1
IMEX max iterations, error:  0  0.000000000000000E+00  0.000000000000000E+00

and create a loop that goes thru comparing results for grep for each pattern as at the string's beginning: " u = ", .... , " TBOT= ", ..., "dKE/dt(W/m^2):". I assume we do not need to compare stuff in init as this should be enough.

oksanaguba commented 1 year ago

this set might be slightly different (amended) for hydrostatic runs

oksanaguba commented 1 year ago

If you guys do not see a more elegant solution, i can do the one from above.

oksanaguba commented 1 year ago

a note to myself -- not sure how cmake interprets \n in strings, need to check that it compares everything, before and after \n's.

ambrad commented 1 year ago

You should be able to use multiple patterns in a single search using | in the pattern string (possibly escaped with \, depending on context).

Also, assign yourself if you're taking this issue and remove the "help wanted" label.

oksanaguba commented 1 year ago

a better solution with grep string built in cmake does not work, but placing the messy long pattern into execute_process() works, so i will make a few blocks of those.

#do not delete this line
SET(GREP_ARGS "\"  u     =")
#SET(GREP_ARGS "${GREP_ARGS}\\|  v     =")
#SET(GREP_ARGS "${GREP_ARGS}\\|  w     =")
#SET(GREP_ARGS "${GREP_ARGS}\\|  T     =")
#SET(GREP_ARGS "${GREP_ARGS}\\|  mu    =")
#SET(GREP_ARGS "${GREP_ARGS}\\|  vTh_dp=")
#SET(GREP_ARGS "${GREP_ARGS}\\|  dz(m) =")
#SET(GREP_ARGS "${GREP_ARGS}\\|  dp    =")
#do not delete this line
SET(GREP_ARGS "${GREP_ARGS}\"")

message("grep_args are ${GREP_ARGS}")

#does not work
#EXECUTE_PROCESS (COMMAND grep ${GREP_ARGS} /home/ac.onguba/ess/runhomme/bfb-output/tests/theta-fhs2-ne2-nu3.4e18-ndays1/theta-fhs2-ne2-nu3.4e18-ndays1_1.out
#works
#EXECUTE_PROCESS (COMMAND grep "  u     =" /home/ac.onguba/ess/runhomme/bfb-output/tests/theta-fhs2-ne2-nu3.4e18-ndays1/theta-fhs2-ne2-nu3.4e18-ndays1_1.out
#does not work
#EXECUTE_PROCESS (COMMAND grep "${GREP_ARGS} /home/ac.onguba/ess/runhomme/bfb-output/tests/theta-fhs2-ne2-nu3.4e18-ndays1/theta-fhs2-ne2-nu3.4e18-ndays1_1.out"
#does not work
#EXECUTE_PROCESS (COMMAND grep "${GREP_ARGS}" /home/ac.onguba/ess/runhomme/bfb-output/tests/theta-fhs2-ne2-nu3.4e18-ndays1/theta-fhs2-ne2-nu3.4e18-ndays1_1.out
#does not work
#EXECUTE_PROCESS (COMMAND grep \"${GREP_ARGS}\" /home/ac.onguba/ess/runhomme/bfb-output/tests/theta-fhs2-ne2-nu3.4e18-ndays1/theta-fhs2-ne2-nu3.4e18-ndays1_1.out
#works
EXECUTE_PROCESS (COMMAND grep "  u     =\|  v     =\|  w     =\|  T     =\|  mu    =\|  vTh_dp=\|  dz(m) =\|  dp    =" /home/ac.onguba/ess/runhomme/bfb-output/tests/theta-fhs2-ne2-nu3.4e18-ndays1/theta-fhs2-ne2-nu3.4e18-ndays1_1.out
                 RESULT_VARIABLE ERROR_CODE
                 OUTPUT_VARIABLE aaa
                 ERROR_VARIABLE  bbb)
MESSAGE("aaa is ${aaa}")
ambrad commented 1 year ago

It's possible your accumulation isn't working because of the \" parts, which I believe are not necessary. The following works for me:

set(foo "sum=")
set(foo "${foo}\\|/dt")
EXECUTE_PROCESS (COMMAND grep ${foo} ...)

To minimize repetition but have a tidy layout of search strings, you might like something of this sort:

set(foo "sum=")
foreach (s
    # list of patterns
    "/dt"
    "f.  ="
    "fq1 ="
    )
  set(foo "${foo}\\|${s}")
endforeach()
bartgol commented 1 year ago

I remember trying to look for a string split over multiple lines, as part of a PASS_REGULAR_EXPRESSION in a test property, and it was a mess, due to all the layers of cmake calls, each of which escaped one layer of \\. I ended up checking a single line. Your idea of wrapping the check in a cmake script, which runs multiple execute_process (each checking a single line) seems like a good solution to me.

Also, I think using the OR in the grep pattern might be not safe, cause it would make grep return immediately after the first match, while we want to check that the whole pattern is there, no? I'm not sure why only one would be found, but hey, you never know with bugs.

ambrad commented 1 year ago

@bartgol I don't think multi-line search is needed here, just multiple patterns. As for returning immediately, that's not an issue: grep runs the whole pattern on each line and emits any line that matches any of the subpatterns. The full output of one grep run is exactly what is needed.