E3sm: Current master (c9903bde) not BFB on Edison: 143 vs 265 nodes

[golaz]

I'm afraid I have some bad news to report (or maybe I did something wrong; always a possibility).

I ran some tests with a recent version of master (c9903bde) using compset A_WCYCL1850S and resolution ne30_oECv3_ICG on Edison. I tried the new PE layouts provided by @jonbob for 143 and 265 nodes. Both 5 day tests ran successfully, unfortunately the results diverge between the two simulations after a few time steps (based on atm.log files). I verified that BFBFLAG is set to true, so my understanding is that I should get the same results.

Here is the run_acme script:
run_acme.20170426.beta1_05.A_WCYCL1850S.ne30_oECv3_ICG.edison

Output on Edison is under:
/global/cscratch1/sd/golaz/ACME_simulations/20170426.beta1_05.A_WCYCL1850S.ne30_oECv3_ICG.edison/test???

[rljacob]

@golaz our nightly testing has revealed a similar problem. We think it was introduced this week and are currently tracking it down (on Slack).

[golaz]

@rljacob - good to hear that this was caught and is being tracked down.

[rljacob]

We found the source of our testing problem. (PR #1272). It has been removed from master which itself will change answers (that was a non-BFB PR). Hopefully that also solves your problem but we're not sure so please try again with the latest version of master.

[golaz]

Thanks, @rljacob and @bishtgautam. Trying now the latest version.

[golaz]

@rljacob : I tried again with (beea721), and unfortunately the results are still not BFB when comparing 133, 143 and 265 nodes.

[rljacob]

Ok. What was the last version of master where this worked for you?

[ndkeen]

Would it be appropriate to try this with DEBUG?

[golaz]

According to my notes, the last time I checked was with b1c676f40 and it worked. But I don't routinely check, as I was assuming this was part of the standard ACME testing procedure.

[mt5555]

based on redsky testing of master, started 2017-04-28 03:52:49
ERP_Ld3.ne30_oEC.A_WCYCL2000.redsky_intel
passed.

So there must be something subtle that A_WCYCL1850S and resolution ne30_oECv3_ICG would not be reproducible, while A_WCYCL2000 ne30_oEC is reproducible.

[golaz]

This is helpful. It's unlikely that this is because of 2000 vs 1850 forcing, so most likely it is due to the use of spun-up ocean and sea-ice initial conditions.

[worleyph]

@golaz, I also see this in my PE layout experiments (now that I look). In particular, only changing the number of OCN processes was sufficient.

[rljacob]

That would suggest the code that does the parallel read/init of the spun-up IC's for the ocean may be an issue.

[rljacob]

Tagging @mark-petersen

[jonbob]

@worleyph - can you check and see if the test fails for A_WCYCL1850 and ne30_oEC60to30v3? That would definitely point to something in reading the initial condition files...

[worleyph]

On titan, with current master, with Intel compiler,

 -compset A_WCYCL2000 -res ne30_oEC

and MPI-only PE layouts that differ only in the number of MPI processes in the OCN (512 -> 256), output in atm.log differs by 'nstep, te 5' .

[rljacob]

Thats a compset/res we test all the time.

[ndkeen]

I'm trying

./create_test PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S
./create_test ERP_Ld3.ne30_oEC.A_WCYCL2000

[worleyph]

My version of master had a "bug fix" in ocn_comp_mct.F and ice_comp_mct.F:

 300c300
 <     call MPAS_io_set_iotype(domain % iocontext, pio_iotype)
 ---
 > !pw    call MPAS_io_set_iotype(domain % iocontext, pio_iotype)

but I also found other recent A_WCYCL cases that show nonreproducible results when changing process count in OCN (and don't have this change).

[rljacob]

ERP_Ld3.ne30_oEC.A_WCYCL2000.redsky_intel passed with hash c9903bde190 from master. ERP is supposed to change the mpi-tasks in all components in the middle of a restart and test BFB.

[worleyph]

My tests all have OCN on its own nodes (as does @golaz 's experiments). I am building a job with components stacked, to see whether this makes a difference.

[ndkeen]

This issue isn't involved right? https://github.com/ESMCI/cime/issues/1433

[rljacob]

Doubtful.

[rljacob]

Pat, try your test with c9903bd. That version had a passing ERP test on redsky but it failed for Chris.

[rljacob]

On Redsky, the ERP_Ld3.ne30_oEC.A_WCYCL2000. test that passed starts with everything stacked on 512 tasks. It then halves them to 256. That should find this bug if it was present in that compset/resolution.

[worleyph]

Not necessarily. My experiment only changed OCN, so if it is the ratio of CPL to OCN processes that matters, then the stacked experiment would not exercise this.

[ndkeen]

Pat what exactly are you running? Must be something smaller/shorter than what I'm trying, which are those tests (265-nodes, still in Q). I did run 5 days of the run_acme script that Chris posted on top. Worked. And now I should change the pe layout, run again to see if it changes results?

Was also going to try running debug with Intel v17 to see if it catches anything obvious.

Adding here that Rob suggests to try:
REP_Ln9.ne11_oQU240.A_WCYCL1850
If that passes, try REP_Ln9.ne30_oECv3_ICG.A_WCYCL1850S
REP does 2 identical initial runs.

[worleyph]

I am running on Titan. This is not Edison-specific. I am running for 1 day.

And now I should change the pe layout, run again to see if it changes results?

Yes, though since I can reproduce directly, not going through the script, perhaps this part of the exercise is not necessary at this point in time. How you change the PE layout apparently matters? I just changed the number of OCN tasks only.

[worleyph]

@rljacob , my 'stacked' experiment was also not reproducible: went from 512x1 to 480x1. I am trying 256x1 next.

[rljacob]

Thanks. Which machine/compiler are you using?

[rljacob]

Nevermind. Titan.

[worleyph]

and Intel compiler.

[rljacob]

Which version of intel?

[worleyph]

intel/15.0.2.164 (the current default on Titan)

[agsalin]

Just trying to follow along -- is everything consistent with the hypothesis that reading in a spun-up ocean (designated by S at end of the compset) is the part that isn't reproducible for different number of PEs?

[worleyph]

Don't think so - I am using a generic A_WCYCL2000 compset.

[rljacob]

Could be that 2 bugs are being chased. Really surprising that Pat is seeing this when ERP_Ld3.ne30_oEC.A_WCYCL2000 passed recently on redsky.

[worleyph]

It is even different comparing 256x1 with 512x1 - which is what ERP_Ld3.ne30_oEC.A_WCYCL2000 should be doing?
I'll try c9903bd. What process counts are used in the ERP_Ld3.ne30_oEC.A_WCYCL2000 test?

[rljacob]

According to the test output (http://my.cdash.org/testDetails.php?test=31255751&build=1185126) it starts with 512. Since "Ld3" is added, it should run for 3 days with 512 and output a restart at day 2. Then pick up that restart with a 256-proc executable and run for 1 day. Then compare output in coupler history files from the end of each run.

[jonbob]

I wish that hypothesis made more sense to me, but I think reading in the spun-up conditions uses the same code as reading in a restart file. I have experiments in mind that I'll try today and tomorrow and see if I can pin it down.

---

From: Andy Salinger [e-mail]
Sent: Friday, April 28, 2017 3:01 PM
To: ACME-Climate/ACME
Cc: Wolfe, Jonathan David; Mention
Subject: Re: [ACME-Climate/ACME] Current master (c9903bde) not BFB on Edison: 143 vs 265 nodes (#1467)

Just trying to follow along -- is everything consistent with the hypothesis that reading in a spun-up ocean (designated by S at end of the compset) is the part that isn't reproducible for different number of PEs?

-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/ACME-Climate/ACME/issues/1467#issuecomment-298105638, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ANDQcG9GAqutY9GfrIsW4Ag0CO8TcmF3ks5r0lOOgaJpZM4NKxU2.

[worleyph]

@rljacob , haven't we run into this before? A nonreproducibility bug that only showed up when comparing two initial runs, and not from a checkpoint? Not that we have verified this, but this sure sounds familiar.

[rljacob]

Indeed we did. We don't have an explicit test for that in the suite but that kind of problem would create an un-expected diff with baselines.

Unless it snuck in with a non-BFB change....

[rljacob]

Everyone try this test: REP_Ln9.ne11_oQU240.A_WCYCL1850

[rljacob]

small and short. If it passes, try REP_Ld3.ne30_oECv3_ICG.A_WCYCL1850S

[worleyph]

I'm stuck in my current tests at the moment (and most of my cost is the compilation). Again, I see differences in the 5th timestep in the atm.log file, so a very short run may be sufficient.

[jonbob]

I have another hypothesis. We fulfilled a PR at the end of February, #1291, that changed restart fields for the ocn. But I checked and the IC file for oEC60to30v3 is older than that and doesn't have the new fields. It could be possible that whatever the ocean is doing in that situation is not consistent over processor counts? I have another set of ICG files I can point to to see if this is responsible.

[worleyph]

Having flashbacks here - I did just now check that two runs with the same layout are the same (through 1 day).

[worleyph]

Repeated the experiments (512x1 and 256x1) using c9903bd and the Intel compiler. Same results - atm.log diverge starting with nstep, te 5. Then tried PGI, and it showed divergence starting with nstep, te 2 ... that was a surprise.

[worleyph]

Ran 512x1 for all components except for OCN, where I used 256x1. This is BFB with the run with all components using 256x1 (for Intel).

[worleyph]

Tried same experiment with PGI (Ran 512x1 for all components except for OCN, where I used 256x1.)

a) This was NOT the same as 256x1 job (unlike for Intel).
b) It was the same as all 512x1 until nstep 5 , so same as Intel result.

Appears that there are at least two issues, one with OCN (PGI and Intel), and one PGI only.

[worleyph]

I tried v1.0.0-beta.1 (00a38722dbce8eaefa690669c1d98bdd11d56154) and it has the same behavior.

[rljacob]

ok. beta.1 tag was made on March 2nd. Chris said the last time he checked and it worked was b1c676f which was Jan 26.

[rljacob]

The commit Jon is concerned about, #1291, was closed and instead its content was merged with #1278. That was hash 012c2659 on March 2nd. The beta1 tag was 3 merge commits later that same day. The commit prior to 1278 is a810dfe6 on Feb 25th so see if that works.

[worleyph]

@rljacob , you put down the same hash twice. What it is the hash before 012c265 that I should try?

[rljacob]

a810dfe6 I'll fix it above.

[jonbob]

I tried changing the ICG files and it failed, so my hypothesis doesn't hold. I'll keep going through the changes we've made over the past few months.

[worleyph]

I forgot to set BFBFLAG to TRUE in my tests of v1.0.0-beta.1. Retrying.

[ndkeen]

REP_Ln9.ne11_oQU240.A_WCYCL1850 passes on edison and cori-knl.

PEM_Ln9.ne4_oQU240.A_WCYCL2000 failed on BOTH edison and cori-knl as one of the tests aborted. Looks like the first job ran, but the 2nd one had this error (both machines had same error):

00: forrtl: error (76): Abort trap signal
00: Image              PC                Routine            Line        Source             
00: acme.exe           0000000004B40131  Unknown               Unknown  Unknown
00: acme.exe           0000000004B3E887  Unknown               Unknown  Unknown
00: acme.exe           0000000004A47B44  Unknown               Unknown  Unknown
00: acme.exe           0000000004A47956  Unknown               Unknown  Unknown
00: acme.exe           00000000049C4E24  Unknown               Unknown  Unknown
00: acme.exe           00000000049D05C1  Unknown               Unknown  Unknown
00: acme.exe           00000000043B2AF0  Unknown               Unknown  Unknown
00: acme.exe           00000000049A52DB  Unknown               Unknown  Unknown
00: acme.exe           0000000004B61B71  Unknown               Unknown  Unknown
00: acme.exe           000000000452BEB2  Unknown               Unknown  Unknown
00: acme.exe           0000000004504091  Unknown               Unknown  Unknown
00: acme.exe           00000000045083C5  Unknown               Unknown  Unknown
00: acme.exe           00000000021BED59  mpas_dmpar_mp_mpa         322  mpas_dmpar.f90
00: acme.exe           000000000253D4AD  mpas_block_decomp         126  mpas_block_decomp.f90
00: acme.exe           000000000216EFB0  mpas_bootstrappin         233  mpas_bootstrapping.f90
00: acme.exe           0000000002EFBB57  ocn_comp_mct_mp_o         461  ocn_comp_mct.f90
00: acme.exe           000000000042075B  component_mod_mp_         227  component_mod.F90
00: acme.exe           00000000004112EC  cesm_comp_mod_mp_        1181  cesm_comp_mod.F90
00: acme.exe           000000000041DA4B  MAIN__                     63  cesm_driver.F90
00: acme.exe           00000000004012CE  Unknown               Unknown  Unknown
00: acme.exe           0000000004B5B761  Unknown               Unknown  Unknown
00: acme.exe           00000000004011B5  Unknown               Unknown  Unknown

OK this is just MPI Abort. In ocn.log, I see:
** Attempting to bootstrap MPAS framework using stream: mesh Bootstrapping framework with mesh fields from input file '/project/projectdirs/ acme/inputdata/ocn/mpas-o/oQU240/ocean.QU.240km.151209.nc' ERROR: Could not open block decomposition file for 24 blocks. Filename: /project/projectdirs/acme/inputdata/ocn/mpas-o/oQU240/mpas-o.graph.info.151209. part.24
And sure enough the .part.24 file is missing. I will create and try again. Failed again because I also needed to add the ice .part.24 file... Submitted.

These are still pending on edison as they are asking for 173 nodes:
ERP_Ld3.ne30_oEC.A_WCYCL2000
REP_Ln9.ne30_oECv3_ICG.A_WCYCL1850S
PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S

On cori-knl, they have poor default pe layouts and timed out -- retrying.

[worleyph]

Both 00a3872 and a810dfe failed for me. Trying b1c676f now.

(IGNORE until reproduced - these were only with the intel compiler, and intel not generally reproducible on Titan currently. See below.)

[worleyph]

Tried b1c676f , and saw nonreproducibility with both Intel and PGI on Titan (changing 512x1 to 256x1 for OCN, everything else fixed at 512x1; with BFBFLAG == TRUE). Feel like I am doing something wrong here ...

[rljacob]

@golaz in your notes on trying b1c676f, what were the 2 pe layouts and was it edison?

[worleyph]

I'm going back to master to determine whether it is just OCN. One test with 00a3872 seems to indicate that it is not ICE.

[ndkeen]

REP_Ln9.ne30_oECv3_ICG.A_WCYCL1850S passes on cori-knl

[rljacob]

Looking at the other times we had problems. ERP was failing back in July. See https://github.com/ACME-Climate/ACME/issues/906. Was fixed by https://github.com/ACME-Climate/ACME/pull/961. Test suite missed it at the time because the exact compset wasn't being tested.

[golaz]

@rljacob : my tests were on Edison with 173 and 375 nodes respectively.

[worleyph]

My one concern is that it wasn't until we set BFB to TRUE that we have been requiring this (i.e. reproducible when changing OCN process counts). I am "confident" that I tested that this was sufficient, in some test with some hash, but I haven't been able to find this again. Most of our work has been to retain this for the atmosphere and land, most recently for atmosphere physics load balancing.

[rljacob]

More recent problem: 2 initial runs were not BFB with threading and intel. https://github.com/ACME-Climate/ACME/issues/1203. This was back in January. Fixed by https://github.com/ACME-Climate/ACME/pull/1219 in hash c1f0a2d30 on Jan 19th.

[rljacob]

Here' the one Pat probably remembers. 2 identical jobs run twice gave different answers: https://github.com/ACME-Climate/ACME/issues/1137 for any coupled case with threading in the ocean. Fixed by https://github.com/ACME-Climate/ACME/pull/1257 in hash 4f3d4f05c on Feb 13th.

[rljacob]

Regarding BFB with changing ocean processors: the ERP test will change the ocean processors and we've had that passing in our test suite for a water cycle case for a while now. That's why we're surprised by this and your observation that its a difference in initial runs, not restart, is likely a clue.

[worleyph]

the ERP test will change the ocean processors and we've had that passing in our test suite for a water cycle case for a while now.

That reassures me - thanks. Going crazy here.

[ndkeen]

The following FAIL on compare for cori-knl. The can go in debug Q, but it's unfortunate they are compiling on the compute nodes.

PEM_Ln9.ne4_oQU240.A_WCYCL2000
PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S

[rljacob]

Ok. Try PEM tests with separate compsets for ocean/ice and atm/land
PEM_Ld3.T62_oQU120.CMPASO-NYF
PEM_Ld3.ne4_ne4.FC5AV1C-04P2

[worleyph]

Note that my latest experiment (Titan, master, intel) comparing all 512x1 with same but ICE 256x1 differed by nstep, te 1, so my earlier comment about ICE being okay does not appear to be true for master. I also did a sanity check, turning ATM load balancing off and on, and this was BFB, so the BFBFLAG==TRUE does appear to be doing its job.

(IGNORE comment on ICE - Intel not generally reproducible on Titan currently. See below.)

[ndkeen]

1) PEM_Ld3.T62_oQU120.CMPASO-NYF
a) passes on cori-knl with intel and gnu
b) passes edison with intel15 and gnu (ran out of time with intel17, trying again)

2) PEM_Ld3.ne4_ne4.FC5AV1C-04P2
a) fails compare on cori-knl with intel (also fails compare forcing 1 thread). does pass with DEBUG (same thing on cori-haswell: fails compare, but passes DEBUG)
b) passes edison with intel15, intel17, and gnu

3) PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S
a) fails compare on cori-knl with intel (also fails compare forcing 1 thread)
b) passes on edison with intel15, but fails compare with GNU (ran out time with intel17, trying again)

4) PEM_Ln9.ne4_oQU240.A_WCYCL2000
a) fails compare on cori-knl with intel (also fails compare forcing 1 thread) (having issues with gnu build, intel DEBUG)
b) passes on edison with intel15, intel17 (gnu build issue)

[worleyph]

Experiments on Titan with master and a baseline of 512x1 for

 -compset A_WCYCL2000 -res ne30_oEC

for a one day simulation and comparing atm.log output.

With PGI:

 a) all 512x1, comparing without and with ATM physics loadbalancing  - BFB
 b) 256x1 LND - BFB
 c) 256x1 ROF - BFB
 d) 256x1 ICE - BFB
 e) 256x1 ATM - BFB
 f) 256x1 CPL - BFB
 g) 256x1 OCN - differs from baseline at nstep, te 5
 h) all 256x1 - BFB with (g) (and differs from baseline at nstep, te 5 compared to baseline)

However (an issue for Titan POC, attention @minxu74 and @mrnorman ) for Intel compiler:

 a) all 512x1, comparing without and with ATM physics loadbalancing - BFB
 b) 256x1 LND - BFB
 c) 256x1 ROF - BFB
 d) 256x1 ICE - differs from baseline at nstep, te 1
 e) 256x1 ATM - differs from baseline at nstep, te 1
 f) 256x1 CPL - differs from baseline at nstep, te 1
 g) 256x1 OCN - differs from baseline at nstep, te 1

So, if we want to continue to use the Intel compiler on Titan, we need to determine less aggressive compiler options if we want reproducibility.

Ignoring all this, the PGI results appear to point at OCN as the source of the code problems?

[worleyph]

Note that most of my evaluations of older commits on Titan were using Intel, since that compiles faster - bad choice. Redoing now with PGI. However, my commit experiment using b1c676f had a PGI experiment, and it showed changing OCN from 512x1 to 256x1 introduced a difference at nstep, te 5, just like current master.

[jonbob]

I ran a bunch of experiments with current master on anvil (and intel), by hand instead of using the test scripts. And this is what I found:

• A_WCYCL1850S and ne30np4_oEC60to30v3_ICG, changing ocn pe-count FAIL
• A_WCYCL1850S and ne30np4_oEC60to30v3_ICG with new ICG files, changing ocn pe-count FAIL
• A_WCYCL1850S and ne30np4_oEC60to30v3_ICG with new ICG files, changing atm pe-count PASS
• A_WCYCL2000S and ne30np4_oEC60to30v3_ICG with new ICG files, changing ocn pe-count FAIL
• A_WCYCL2000 and ne30np4_oEC60to30v3, changing ocn pe-count FAIL
• GMPAS-IAF and T62_oEC60to30v3, changing ocn pe-count FAIL

It certainly makes it look like the ocean has an issue. I'll try turning off and on some of the recent additions we've made and see if I can pinpoint anything..

[worleyph]

@jonbob , note that I see the problem as far back as b1c676f , so Jan. 26 .

[jonbob]

Thanks @worleyph - I'll try to see what happened back then

[rljacob]

This is encouraging that its narrowing down the problem. A little discouraging that Chris said b1c676f passed for him.
@ndkeen your results suggest there might be a separate problem specific to cori-knl and an F-case. Also try the test that should mimic what Jon saw: PEM_Ld3.T62_oEC60to30v3.GMPAS-IAF

[worleyph]

I've continued to try earlier and earlier commits. Oldest so far was b520988 (commited on Dec. 19) and it still shows the problem. Might be easier to just try to debug this directly? I tried to build a CIME2 version, but the build failed for an obscure (to me, CIME related?) problem.

[ndkeen]

PEM_Ld3.T62_oEC60to30v3.GMPAS-IAF

Run failure on edison and it wrote these 2 log files:

```edison07% tail run/log.004*
==> run/log.0045.abort <==
MPAS-seaice: IR negative reconstructed ice area (nDims == 2)

==> run/log.0047.abort <==
MPAS-seaice: IR negative reconstructed ice area (nDims == 2)

Also trying to run these DEBUG and hitting some annoying runtime error in OCN/ICE -- I'm fixing them along the way, but I don't think they are related to the issue.

example:

17: forrtl: severe (194): Run-Time Check Failure. The variable 'cice_analysis_driver_mp_cice_precompute_analysis_members_$ERR_TMP' is being used in 'mpas_cice_analysis_driver.f90(1267,7)' without being defined
```

I think my cori-knl GNU build issues might be happening because of the attempted building on compute nodes. It looks like there are two tests and two different builds. The first one happens on login node and is OK.

[jonbob]

@ndkeen - unfortunately, that compset doesn't yet get correct settings out of the box. You'll need to set ATM_NCPL to 2 and it should run fine

[rljacob]

Given the new issue Pat opened for Titan/Intel ( #1472 ) I guess we should not use that machine/compiler combo to diagnose this issue.

[ndkeen]

I don't know how to make an xmlchange using create_test. Esp if the test is building/running 2 cases to compare.

[rljacob]

Chris said he " tried the new PE layouts provided by @jonbob for 143 and 265 nodes". Jon, what exactly are those?

[jonbob]

OK, I ran some more test overnight and think I know where the problem is. I'll talk to the developers once I get in and see if we can pinpoint the solution.

[jonbob]

@rljacob - I've been documenting them on #1387, as well as providing them to Chris and Peter and Pat

[ndkeen]

Jon: do you have the wrong # there? #1387 refs an older edison slowdown issue.

[jonbob]

@ndkeen - No, that's where we had discussed improved edison pe-layouts

[ndkeen]

OK, just didn't see any new updates so I thought it was an old one.

[jonbob]

Testing on edison, I haven't been able to recreate the PASS I thought I got on anvil this morning. So until anvil/blues comes back, I won't know for sure if we know where the problem is....

[jonbob]

We've completed more tests and all indicators point to an issue in the ocean. I've been checking a few other parts of the model, by toggling parameters, and can say the following:

• DEBUG runs also fail -- so not likely any optimization issue;
• not in CVMIX;
• not in GM.
  We're looking through commits over the past several months, and I'll test older branches/tags and see if we can narrow down what brought this problem into the repo.

[mark-petersen]

In MPAS-O stand alone, I get bfb match between 4 and 8 blocks on the QU240, with either intel or gnu. @jonbob on the tests that failed in ACME with EC60to30v3, what are the number of blocks (i.e. graph files) you are using for the two runs?

[jonbob]

@mark-petersen - I've tried several different decompositions, but right now am using 240 and 480. For what it's worth, oQU240 G cases pass in ACME with 24/48 as well -- so it's something specific to oEC60to30v3. For the oEC60to30v3 runs, I tried

 config_hmix_scalewithmesh = .false.

which is the only obvious namelist difference between the two resolutions -- but that also fails. Otherwise, I only see timestep differences. Can you think of anything else?

[worleyph]

@jonbob , note that I see the same issue (as far as I can tell) with just oEC60to30 (no v3).

 ice/mpas-cice/oEC60to30/mpas-cice.graph.info.151020
 ocn/mpas-o/oEC60to30/mpas-o.graph.info.151031

[jonbob]

We can verify that we see this same problem in standalone MPAS-O. We also have the oQU240 grid in our automated tests, and for as yet unknown reasons, the BFB test passes oQU240 and fails oEC60to30 (and its variants). I tested as far back as master from early Oct, 2016 and it was still apparent at that time. @mark-petersen is tracking this down in the standalone model and will update this page as soon as he finds anything.

[rljacob]

Any idea why the ERP_Ld3.ne30_oEC.A_WCYCL2000 test did not catch this? that test: (1) Uses the oEC60to30 grid (2) uses MPAS-O (3) changes the mpi-tasks for mpas-o between an initial and restart run (4) checks BFB between the common end points of each run.

It only checks coupler history. Does it not show up there?

[mark-petersen]

This fixes the bfb block mismatch in MPAS-O stand alone:

components/mpas-o/model/src/core_ocean/mode_forward/mpas_ocn_time_integration_split.F
line 1019
- if ( edgeHaloComputeCounter == 1 .and. BtrCorIter /= config_n_btr_cor_iter ) then
+ if ( edgeHaloComputeCounter == 1 .or. BtrCorIter == 1 ) then
...
call mpas_dmpar_field_halo_exch(domain, 'normalBarotropicVelocitySubcycle', timeLevel=newBtrSubcycleTime)

This forces an extra halo update on the split explicit barotropic subcycling (2D fields). It should fix the problem in ACME as well. If any of you could try this in the ACME configuration where you saw the block mismatch, that would be helpful.

This fix should not be needed for a 3-cell-wide halo around each block. It could also be fixed by requesting a 4-wide halo in the ocean namelist:
config_num_halos = 4
but that would increase the communication for all halo updates (2D and 3D) and would slow the simulation, so I prefer the first fix above.

Doug and I reduced the halo computations in https://github.com/MPAS-Dev/MPAS/pull/998 in August 2016. After reevaluating today, I still think the current repository is correct for a 3-wide halo. I suspect that the EC60to30 does not have a true three-wide halo for some partitions - perhaps caused by an unusual 5 or 7-sided polygon in the halo, so the halo rings are not counted correctly. If this hypothesis is correct, that would explain why it occurs for some resolutions and partitions, but not for others. It would also occur more commonly at higher resolutions, where your odds of having an unusual cell in a halo is higher.

The fix at the top of this post is a sure-fire solution. My guess is it will slow the code by 0.3 to 0.5%, as the total speed-ups from https://github.com/MPAS-Dev/MPAS/pull/998 was 1.7%, and that PR included several other halo reductions. I will look closer at the halo regions for the EC60to30, and see if I can find incorrect halo widths.

[worleyph]

@mark-petersen , does this fix a real bug (and not just eliminate a nonreproducible aspect of the algorithm)? Any impact on the ongoing simulations?

[worleyph]

@mark-petersen , in my first experiment (Titan, PGI, changing 512x1 PE layout to 256x1), this change did not eliminate the problem. In fact, the results are bfb the same with the runs without this change. I checked my build directory, and the change is in there.

Looking at the timing files, the timer "se halo btr vel corr" does not show up, so this code is not being executed. The parent timer "btr se subcycle loop" is called, as well as a 'sibling' timer "se halo subcycle".

[mark-petersen]

Strange. Just comment out the if statement completely, so that the halo update is always executed. I added a print statement as well so I can see what is going on.

components/mpas-o/model/src/core_ocean/mode_forward/mpas_ocn_time_integration_split.F
line 1019
! if ( edgeHaloComputeCounter == 1 .and. BtrCorIter /= config_n_btr_cor_iter ) then
    print *, 'MPAS BTR VEL HALO: edgeHaloComputeCounter, BtrCorIter', edgeHaloComputeCounter, BtrCorIter
    edgeHaloComputeCounter = config_num_halos + 1
    call mpas_timer_start("se halo btr vel corr")
    call mpas_dmpar_field_halo_exch(domain, 'normalBarotropicVelocitySubcycle', timeLevel=newBtrSubcycleTime)
    call mpas_timer_stop("se halo btr vel corr")
! else
!    edgeHaloComputeCounter = edgeHaloComputeCounter - 1
! end if

[worleyph]

@mark-petersen , commenting out the 'if' led to reproducible runs for my one example. And the timer "se halo btr vel corr" does show up now.

[jonbob]

@mark-petersen - I have test in on edison as well -- and can at least get the output from the print statement, as well as try to confirm @worleyph's results

[mark-petersen]

Great. @worleyph could you paste a few lines from that print statement? BtrCorIter should just count 1:2.

[worleyph]

Sorry - I did not include the print statement - was confused and thought that was just something you were going to use in your tests. I'll try again, or perhaps @jonbob has this information.

[mark-petersen]

@worleyph in response to your comment this morning "Does this fix a real bug".

Forcing extra halo updates will produce the correct result, and ensure bfb match between different partitions. However, I believe this fix is heavy handed - we shouldn't need to update the halo here if we really have a three-wide halo ring. So the fix in not optimal to minimize communication.

My proposed plan is:

1. The extra halo update should be tested in ACME on separate ocean block partitions to make sure this is the actual problem.
2. Today I will assess if the EC60to30 partitions have some errors in constructing their three-wide halos.
3. If I can find an error in the MPAS halo init quickly, that would be the better thing to fix. If not, adding an extra halo update will produce the correct results. This will break bfb on the nightly ACME test for some resolutions.
4. I suspect the halo initialization will take longer to understand and fix (days to a week). We could change from the extra halo update to fixing the halo init later, and it would be a bit-for-bit change.

[jonbob]

My runs have the output -- they're in the debug queue on edison and should go soon

[mark-petersen]

@worleyph when you changed this line on your first test this morning:

- if ( edgeHaloComputeCounter == 1 .and. BtrCorIter /= config_n_btr_cor_iter ) then
+ if ( edgeHaloComputeCounter == 1 .or. BtrCorIter == 1 ) then

are you sure you changed the .and. to an .or.?

[worleyph]

@mark-petersen - sorry - my question was too brief and informal. There is a difference (in my mind) between two simulations that differ because of differences in order of operations (so nonreproducible, but at the level of round-off) and simulations that differ because they are doing the wrong thing - using non-updated halo data. Sounds like this issue is of the latter sort, though using data a little out of date in a very few locations probably does not do much harm. Does change the numerical algorithm however?

[worleyph]

@mark-petersen - I did miss the "and" to "or" change. I'll try again. Sorry. However, the original code was never executing the halo updates, so probably something still wrong with this?

[mark-petersen]

@worleyph the missing 'or' would explain why it never ran. That is important, because removing the if completely doubles the number of halo updates in that spot.

From what I see, the current bug that this fixes is a real error, i.e. the domain is getting bad information from the boundaries.

[worleyph]

@mark-petersen , looks like your fix is working. Sorry for the reporting failure and making you do extra work. It's early on, and I will confirm once the experiments are complete.

[jonbob]

Worked for me on edison as well. I'll dig up output from the ocn log and post it momentarily

[jonbob]

The output from the print statement looks like:

000:  MPAS BTR VEL HALO: edgeHaloComputeCounter, BtrCorIter           3           1
000:  MPAS BTR VEL HALO: edgeHaloComputeCounter, BtrCorIter           4           2
000:  MPAS BTR VEL HALO: edgeHaloComputeCounter, BtrCorIter           3           1
000:  MPAS BTR VEL HALO: edgeHaloComputeCounter, BtrCorIter           4           2

[jonbob]

@rljacob - I don't know why the ERP test didn't catch this bug. If it's working as advertised, it should. The difference shows up in cpl restart files for sure, but I don't know about cpl history output.

[mark-petersen]

My suspicion is that for some partitions and resolutions, a few partitions have a spot with a 2-wide halo by mistake, rather than 3-wide halos everywhere. I will confirm that soon. My bet is that a 5 or 7 sided cell causes this problem, but probably 98% of the cells are 6 sided.

If I am correct, then failing a block decomposition test only depends whether one of these trouble-maker cells happened to fall into a halo. Just to be clear, the cell itself is fine, but there may be a bug in the halo ring counting code upon initialization in MPAS.

[philipwjones]

If it makes you all feel better... Long ago, when POP roamed the land, I introduced a halo optimization (an EW sweep, followed by a NS sweep) and thought I had a pretty rigorous test code that didn't pick up any problems. But it turned out that if an all-land block managed to hit a corner (and only a corner) of another block's halo, it failed. This was an extremely rare pathology/decomposition, but one production configuration just happened to have it. It only takes one and you can't catch everything... This thread has been a bit of deja vu for me and will probably require some therapy.

[rljacob]

@worleyph are you still looking at atm nstep output to determine reproduceability?

[worleyph]

@rljacob , yes, atm.log nstep output is all that I have been looking at.

[rljacob]

@jonbob are these the two decomps where Chris originally found the problem: https://github.com/ACME-Climate/ACME/issues/1387#issuecomment-294019074

[jonbob]

@rljacob - yes, I believe so, though he was also testing a slightly smaller layout from @worleyph

[worleyph]

My recent experiments have all been very small node count layouts, on Titan (32 nodes and 16 nodes). I did see the problem on Edison (when I went back to look) for both 173 node and 133 node PE layouts (my attempt to improve on @jonbob 's work).

[rljacob]

If it starts in the ocean and shows up in the atmosphere, it must be in the coupler.

For the ERP_Ld3.ne30_oEC.A_WCYCL2000 test, skybridge tests 128 and 64 mpi tasks counts. Redsky tests 512 and 256. Blues tests 1024 and 512. Do those 3 pairs of task counts and their associated decompositions not provoke this bug?

[jonbob]

@rljacob - they should. Can you compare cpl restart files instead of history files and see if that catches it? Though I agree, if it shows up in the atm it should be in cpl history as well. Any chance the test is not working as it should?

[mark-petersen]

See #1487. @worleyph and @jonbob please do any further testing on that PR, which should be the same as what you have been testing today.

[worleyph]

@mark-petersen , I've run out of time to do any more testing at the moment. Hopefully @jonbob and others can take a look at this. Thanks for figuring this out.

[rljacob]

cpl history actually has more fields then restart. Pat said 32 and 64 will fail so I'll try an ERP test with that and check the nstep values too.

[rljacob]

The pairs of testing configs I mentioned above where mpi tasks counts. Pat, did you mean 32 and 64 mpi-tasks show difference?

According to this: https://github.com/ACME-Climate/ACME/issues/1387#issuecomment-294019074 edison had 960 vs. 720 mpi-tasks for the ocean showing a difference. I'd like something smaller that definitely fails.

[worleyph]

No - I was using 256 and 512. Doesn't mean that smaller won't show it, but this was a balance to get something that would run under 30 minutes for 1 day and getting through the debug queue quickly. I didn't spend any time looking for anything smaller.

[mark-petersen]

In MPAS stand-alone, the EC60to30 failed to match between 8 and 16 partitions after one step. It should fail to match in ACME as well, if you want something that small. The graph files are in the repo:

inputdata/ocn/mpas-o/oEC60to30v3/mpas-o.graph.info.161222.part.8
inputdata/ocn/mpas-o/oEC60to30v3/mpas-o.graph.info.161222.part.16

That is, they failed before this PR, and match after this PR.

[rljacob]

Thanks, Mark. I assume you are doing 2 initial runs. If the 8 pe run started with a restart from the 16 pe run, would the results be different (without the bug fix)?

[mark-petersen]

Yes, my test was to run two simulations, an 8 pe and 16 pe, from the same initial condition, both for a single time step, and compare the output.

In your case, your run: 16 pe -> restart with 8pe. What are you comparing to? If it is a full duration, either 8 or 16 pe, they should indeed not match.

[rljacob]

Suppose you run 10 steps with 16 pe and write a restart at step 5. Pick up that restart with 8pe and run 5 steps. The values at the end for each run should match. But with the bug, they won't match?

[mark-petersen]

Correct. They should: mismatch before this PR, match after.

[rljacob]

One more clarification: will any of the oEC60to30 grids show this or only oEC60to30v3?

[mark-petersen]

Any. @worleyph tried a previous one, and it had a bfb mismatch between different partitions. The versions only differ in how they treat single-cell wide channels.

[ndkeen]

fyi, I added this change and
PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S now passes on cori-knl (was failing compare before)

However, PEM_Ln9.ne4_oQU240.A_WCYCL2000 still fails compare.

[mark-petersen]

Has PEM_Ln9.ne4_oQU240.A_WCYCL2000 ever passed compare? I suspect it is a different problem than the one we just fixed here.

BTW, the documentation has the wrong comparison case:

vi cime/scripts/lib/CIME/SystemTests/README
PEM    modified pe counts mpi bfb test (seq tests)
       do an initial run with default pe layout                               (suffix: base)
       do another initial run with modified pes (NTASKS_XXX => NTASKS_XXX/2)  (suffix: modpes)
       compare base and single_thread

last word should be modpes. Same with
cime/config/config_tests.xml line 87
but it is correct in the actual test setup at vi cime/scripts/lib/CIME/SystemTests/pem.py.

[rljacob]

Yes that would be different problem. What machine, compiler and hash did that test fail on?

[ndkeen]

This failure is on cori-knl hash=beea721
The only changes I've made are the ones above.

PEM_D_Ld3.T62_oQU120.CMPASO-NYF passes (as it always has)

And now I'm seeing that this test is now passing where it was failing before the change.
PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S

Actually, I did make one other change. I added an entry for cori-knl to have a more reasonable pe layout for the ne30 case (otherwise default was like 4 nodes). I used one similar to edison and updated slightly to account for 64 cores per node.

+  <grid name="a%ne30np4">
+    <mach name="cori-knl">
+      <pes compset="CAM5.+CLM45.+MPASCICE.+MPASO.+MOSART.+SGLC.+SWAV" pesize="any">
+        <comment>"first try"</comment>
+        <ntasks>
+          <ntasks_atm>5400</ntasks_atm>
+          <ntasks_lnd>640</ntasks_lnd>
+          <ntasks_rof>640</ntasks_rof>
+          <ntasks_ice>4800</ntasks_ice>
+          <ntasks_ocn>3600</ntasks_ocn>
+          <ntasks_glc>640</ntasks_glc>
+          <ntasks_wav>4800</ntasks_wav>
+          <ntasks_cpl>4800</ntasks_cpl>
+        </ntasks>
+        <nthrds>
+          <nthrds_atm>1</nthrds_atm>
+          <nthrds_lnd>1</nthrds_lnd>
+          <nthrds_rof>1</nthrds_rof>
+          <nthrds_ice>1</nthrds_ice>
+          <nthrds_ocn>1</nthrds_ocn>
+          <nthrds_glc>1</nthrds_glc>
+          <nthrds_wav>1</nthrds_wav>
+          <nthrds_cpl>1</nthrds_cpl>
+        </nthrds>
+        <rootpe>
+          <rootpe_atm>0</rootpe_atm>
+          <rootpe_lnd>4800</rootpe_lnd>
+          <rootpe_rof>4800</rootpe_rof>
+          <rootpe_ice>0</rootpe_ice>
+          <rootpe_ocn>5440</rootpe_ocn>
+          <rootpe_glc>4800</rootpe_glc>
+          <rootpe_wav>0</rootpe_wav>
+          <rootpe_cpl>0</rootpe_cpl>
+        </rootpe>
+      </pes>
+    </mach>
+  </grid>

[rljacob]

@ndkeen ok. Thanks for finding PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S! I was looking for a test that could test this bug.

[ndkeen]

Note a run just finished on edison (4 days in Q) and I updated an old comment where I was trying to organize this -- the GNU build of PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S is failing compare. This would be before this change. Should I try it again with the change?

[rljacob]

Yes please.

[ndkeen]

OK, verified that this fix also allows PEM_Ld3.ne30_oECv3_ICG.A_WCYCL1850S to pass on edison with GNU