Skip to content

Reproducibility problem with cnmatrix first occurs in dev172 going from ccs_config_cesm0.0.88 to ...89 #2619

Open
0 of 1 issue completed

Description

@slevis-lmwg

A cnmatrix reproducibility problem first occurs in dev172 and more specifically when going from ccs_config_cesm0.0.88 to ccs_config_cesm0.0.89. This is the only difference that I see when comparing 89 to 88:

git diff ccs_config_cesm0.0.88 ccs_config_cesm0.0.89

diff --git a/machines/derecho/config_machines.xml b/machines/derecho/config_machines.xml
index 633fe74..6f9d6fd 100644
--- a/machines/derecho/config_machines.xml
+++ b/machines/derecho/config_machines.xml
@@ -21,11 +21,11 @@
     <MPI_GPU_WRAPPER_SCRIPT>get_local_rank</MPI_GPU_WRAPPER_SCRIPT>
     <PROJECT_REQUIRED>TRUE</PROJECT_REQUIRED>
     <mpirun mpilib="default">
-      <executable>mpiexec</executable>
+      <executable>mpibind</executable>
       <arguments>
         <arg name="label"> --label</arg>
-        <arg name="buffer"> --line-buffer</arg>
-        <arg name="num_tasks" > -n {{ total_tasks }}</arg>
+       <arg name="buffer"> --line-buffer</arg>
+       <arg name="separator"> -- </arg>
       </arguments>
     </mpirun>
     <module_system type="module" allow_error="true">

Originally posted by @slevis-lmwg in #640 (comment)

Definition of Done:

  • Add warning to build-namelist to abort when matrix is on and OMP_NUM_THREADS > 1 and test it
  • Have our tests setup to ignore the warnings for our tests for this configuration and test it
  • Add a test to the build_namelist_unit_tester with OMP_NUM_THREADS = 2 and verify that it aborts as intended
  • Use NTHRDS_LND rather than OMP_NUM_THREADS
  • Fix CN Matrix code so that OMP_NUM_THREADS env variable is NOT used, but use values from the NUOPC cap
  • Make sure that fixes it -- and if not track down the problem
  • Fix the real problem
  • Remove the warning in the namelist

Activity

  1. added
    nextthis should get some attention in the next week or two. Normally each Thursday SE meeting.
    on Jun 24, 2024
  2. slevis-lmwg commented on Jun 24, 2024

    @slevis-lmwg
    ContributorAuthor

    The original aux_clm failure:
    ERP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn BASELINE ctsm5.2.005.cn-matrix_n09: DIFF
    The same test without --clm-matrixcnOn passed and the two tests differ ONLY as follows:

    <  hist_fincl1 = 'TRAFFICFLUX','SNOWLIQ:A','SNOWICE:A','FCO2'
    ---
    >  hist_fincl1 = 'TRAFFICFLUX', 'SNOWLIQ:A', 'SNOWICE:A', 'FCO2', 'SNO_EXISTENCE', 'SNO_ABS', 'SNO_T:M', 'SNO_GS:X'
    33,34c33,34
    <  hist_nhtfrq = -24,-8
    <  hist_wrt_matrixcn_diag = .true.
    ---
    >  hist_nhtfrq = 0,-240
    >  hist_wrt_matrixcn_diag = .false.

    @ekluzek explains that this hist_wrt_matrixcn_diag output is important for SASU spin-ups.

    We changed the ERP test to a REP test to show that this is a threading reproducibility problem:

    FAIL REP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn COMPARE_base_rep2
    PASS REP_P128x1_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn
    

    This is the failure that first appears in dev172 with ccs_config_cesm0.0.89.

    From #640 testing/troubleshooting on this issue:
    git grep matrixcnOn | grep 'test name' | grep P64x2 on the testlist and try these tests as REP tests. Change the Lm13 test (that we know fails) with Ld13 to see if shorter also fails. They all passed.

    PASS REP_P64x2_Ld10_D.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-ciso_decStart--clm-matrixcnOn
    PASS REP_P64x2_Ld10_D.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-default--clm-matrixcnOn
    PASS REP_P64x2_Ld3_D.f10_f10_mg37.I1850Clm50BgcCrop.derecho_intel.clm-default--clm-matrixcnOn
    PASS REP_P64x2_Ld3_D.f10_f10_mg37.I2000Clm60BgcCrop.derecho_intel.clm-coldStart--clm-matrixcnOn
    PASS REP_P64x2_Ld3_D.f10_f10_mg37.I2000Clm50BgcCru.derecho_intel.clm-flexCN_FUN--clm-matrixcnOn
    PASS REP_P64x2_Ld3_D.f10_f10_mg37.I2000Clm50BgcCru.derecho_intel.clm-noFUN_flexCN--clm-matrixcnOn
    PASS REP_P64x2_Ly3.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-cropMonthOutput--clm-matrixcnOn
    PASS REP_P64x2_Ld5_D.f10_f10_mg37.I1850Clm50Bgc.derecho_intel.clm-ciso--clm-matrixcnOn
    PASS REP_P64x2_Ld13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn
    PASS REP_P64x2_D.f10_f10_mg37.I1850Clm50BgcCrop.derecho_intel.clm-default--clm-matrixcnOn
    
  3. ekluzek commented on Jun 24, 2024

    @ekluzek
    Contributor

    The change has to do with how MPI is binding processors on Derecho. This must have some influence on how the code is running for threading. There were problems with the code before the binding was done, which might mean that it passed before, because threading wasn't working correctly. And now we are seeing bad behavior with threading, but it's probably because threading is actually working correctly with this update.

    #2296 #2289

    Also links to earlier mentions of potential threading problems in the cnmatrix PR:
    #640 (comment)
    #640 (comment)

  4. changed the title [-]Reproducibility problem with cnmatrix first occurs in dev172 going from ccs_config_cesm0.0.88 to ccs_config_cesm0.0.89[/-] [+]Reproducibility problem with cnmatrix first occurs in dev172 going from ccs_config_cesm0.0.88 to ...89[/+] on Jun 24, 2024
  5. ekluzek commented on Jun 24, 2024

    @ekluzek
    Contributor

    Note there are some issues with threading for CN-Matrix.

    1. Threading code uses different calls than without it
    2. Matrix is using the OMP_NUM_THREADS env variable rather than what's passed down from the NUOPC cap

    The first shows potential issues in how the code was implemented, but not likely something easy to fix now. The second is something that it seems we should fix though.

  6. removed
    nextthis should get some attention in the next week or two. Normally each Thursday SE meeting.
    on Jul 8, 2024
  7. self-assigned this
    on Jul 9, 2024
  8. moved this from Todo to In Progress in LMWG: Sprint Planning Boardon Jul 9, 2024
  9. slevis-lmwg commented on Jul 11, 2024

    @slevis-lmwg
    ContributorAuthor

    @ekluzek I committed our changes (commit 4ca2b6a), but they do not work. I troubleshooted a bit, and all I could get was: out=NTHRDS_LND = 0

    Testing with:
    ./create_test REP_P64x2_Ld13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn

  10. slevis-lmwg commented on Jul 11, 2024

    @slevis-lmwg
    ContributorAuthor

    @ekluzek by looking elsewhere in the code and through repeated trial and error, I have managed to solve this to my pleasant surprise.

  11. slevis-lmwg commented on Jul 12, 2024

    @slevis-lmwg
    ContributorAuthor

    I'm working on the next TODO item and having no luck. I pushed my attempt to 8e618bb in case @ekluzek you have suggestions.

    The error says

    #   Failed test 'matrixcnOn_with_threading'
    #   at ./build-namelist_test.pl line 1393.
    #          got: '0'
    #     expected: anything else
    
  12. ekluzek commented on Jul 15, 2024

    @ekluzek
    Contributor
    1. So the problem is that OMP_NUM_THREADS is a proper UNIX environment variable and NOT part of the env_run.xml file. So it has to be treated in a different way from the env_*.xml file variables.

    2. The other problem with using NTHRDS_LND is that it's implemented in a more complex way in the XML file with a full entry like this:

        <entry id="NTHRDS">
          <type>integer</type>
          <values>
            <value compclass="ATM">1</value>
            <value compclass="CPL">1</value>
            <value compclass="LND">1</value>
    .
    .
    .
            <value compclass="ESP">1</value>
          </values>
          <desc>number of threads for each task in each component</desc>
        </entry>

    It's treated as NTHRDS_LND by xmlchange and xmlquery, but the perl unit tester AND the perl build-namelist code would need to implement the above syntax in order to do this correctly. This likely would take a bit to figure this out. So dropping for now.

    We figure we will want to implement the first option so that the unit tester is testing this capability. But, it will take a little more finagling than we want to spend now. But, is something that's straightforward to do (at least for me).

  13. 23 remaining items

  14. slevis-lmwg commented on Sep 4, 2026

    @slevis-lmwg
    ContributorAuthor

    You see this passing since ctsm5.4.025, and the tests in #3360 that pass on resubmit are already not present in ExpectedFails, so I propose we go ahead and remove these, too. @ekluzek what do you think? And do you prefer to discuss on Thursday? I will add next.

  15. added
    nextthis should get some attention in the next week or two. Normally each Thursday SE meeting.
    on Sep 4, 2026
  16. billsacks commented on Sep 4, 2026

    @billsacks
    Member

    Also, ERP_P64x2_Ld396.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings is failing COMPARE_base_rest in my testing. From looking at baselines it looks like this has been failing since ctsm5.4.051. It isn't in the ExpectedFails list, but it does show an expected BASELINE failure, and other similar tests list COMPARE_base_rest in the expected fails list.

    I'm thinking this test should also have COMPARE_base_rest added to the ExpectedFails list. Do others agree?

  17. slevis-lmwg commented on Sep 4, 2026

    @slevis-lmwg
    ContributorAuthor

    Seems logical, but I will let @ekluzek decide.

    I acknowledge that this one slipped by me twice because I have trained my eyes to look for a second FAIL (I should have checked for RUN FAIL and didn't in this case) to justify the COMPARE_base_rest failures not listed as expected...

  18. ekluzek commented on Sep 5, 2026

    @ekluzek
    Contributor

    Also, ERP_P64x2_Ld396.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings is failing COMPARE_base_rest in my testing. From looking at baselines it looks like this has been failing since ctsm5.4.051. It isn't in the ExpectedFails list, but it does show an expected BASELINE failure, and other similar tests list COMPARE_base_rest in the expected fails list.

    I'm thinking this test should also have COMPARE_base_rest added to the ExpectedFails list. Do others agree?

    Yes absolutely. We want the expected fails to accurately show these kind of details correctly.

    Thanks for noticing and getting this right.

  19. removed
    nextthis should get some attention in the next week or two. Normally each Thursday SE meeting.
    on Sep 10, 2026
  20. ekluzek commented on Sep 10, 2026

    @ekluzek
    Contributor

    I'm pretty sure @billsacks added the needed PEND status in ctsm5.4.055.

  21. billsacks commented on Sep 10, 2026

    @billsacks
    Member

    I'm pretty sure @billsacks added the needed PEND status in ctsm5.4.055.

    Yes, I did

  22. slevis-lmwg commented on Oct 2, 2026

    @slevis-lmwg
    ContributorAuthor

    Recording here that in generating the ctsm5.4.062 and 063 baselines I got:
    PASS ERP_P64x2_Ld396.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings COMPARE_base_rest (UNEXPECTED: expected FAIL)
    I will not change it back, yet.

    The REP test still failed.

  23. slevis-lmwg commented on Oct 13, 2026

    @slevis-lmwg
    ContributorAuthor

    I have decided to remove
    ERP_P64x2_Ld396.f10_f10_mt201.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings
    from expected failures in PR #4234. It has consistently passed in my testing recently.

  24. slevis-lmwg commented on Oct 22, 2026

    @slevis-lmwg
    ContributorAuthor

    Funny (as in peculiar and not so funny) in #4234:
    FAIL ERP_P64x2_Ld396.f10_f10_mt201.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings BASELINE ctsm5.4.066_PR4234: DIFF
    Note this is a BASELINE diff, rather than COMPARE_base_rest.
    I will not mark expected, yet, until I get to final testing for #4234.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugsomething is working incorrectlypriority: lowBackground task that doesn't need to be done right away.scienceEnhancement to or bug impacting science

Type

Projects

Relationships

None yet

Development

No branches or pull requests

Issue actions