Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jgfouca/upstream merge 2024 11 14 #3115

Merged
merged 288 commits into from
Nov 15, 2024
Merged

Conversation

jgfouca
Copy link
Member

@jgfouca jgfouca commented Nov 14, 2024

@dqwu Asked for this. This should be one of the last upstream merges.

erinethomas and others added 30 commits September 9, 2024 15:49
compute rof lnd interaction only if rof_c2_lnd is true
we were computing an unnecessary map always
even if rof_c2_lnd is false
(for most coupled cases it is true)
in case rof c2 lnd, we were not retrieving the the MPI group
needed for comm graph computation, for same grid path
we were retrieving it for intx only
how come we never had samegrid land rof before ?
Enables two major changes to the parameterization that calculates LAI
burial by snow: a) pulls stocking density and taper parameters that
influence vegetation height from hard-wired values in
biogeochem/VegStructUpdateMod.F90 to allow to be specified on the
surface file (per main/pftvarcon) and b) updates the LAI burial by snow
rate from Wang and Zeng, 2007 (linear burial) to form proposed
by Liston and Hiemstra, 2011 (linear or non-linear) burial. The Liston
and Heimstra (2011) formulation introduces two new parameters:
1) bendresist, which specifies how resistant branches are to bending
under snow loading (1 = no bending, lim->0 branches bend very rapidly)
and 2) vegshape, which specifies how the shape of the vegetation crown
affects how rapidly it is buried by snow (1 = paraboloid; 2 = hemispheroid).
These new parameters can also be specified on surface file.
Temporary change, pending merge of IM4 updates. IM4
removed a lot of the hard-coded tests that looked
for particular pfts by their assigned number. this required
modifying the helper function "woody" to be a non-binary
(i.e., now 0 = herbaceous, 1 = woody tree, 2 = woody shrub),
but this updated function isn't available to IM3 currently.
The taper array was being initialized incorrectly,
leading to developer test failures. This moves
the definition down further to be able to initialize
correctly.
Adds a new exact restart test that uses new parameters
in the param file for snow/vegetation interactions.
fixes an issue where using less than the maximum pfts
set new parameters with non-physical values
…ing and continue (instead of failing) when input has month dimension and output does not.
jayeshkrishna and others added 11 commits November 11, 2024 12:06
Adding support for ADIOS builds that include support for BLOSC
(lossless compression) in FindPIO cmake module
…6739)

[BFB]

* jgfouca/scream_downstream_2024_11_07: (182 commits)
  Add ghci-oci
  Workflows: reworked how to skip eamxx testing jobs
  Workflows: added more eamxx workflows
  Workflows: fix logic for eamxx jobs skipping
  Workflows: allow to generate ALL baselines at ones for eamxx
  EAMxx: Use the main branch of mam4xx.
  EAMxx: Add tracer_reader_utils.hpp to microphysics.hpp.
  EAMxx: Remove temporary CPP macro.
  EAMxx: Update description of dry aerosol particle diameters.
  EAMxx: Use a temporary branch for mam4xx to test changes in the photo table code.
  EAMxx: Move read utils files to readfiles folder.
  EAMxx: Refine linoz parameter handling and class member references
  Correcting data types for variables, refining comments, and updating YAML input configurations.
  Addresses review comments: adds doc strings, reverts scorpio file change and cleanup
  Adds emissions file for ne4pg2, removes qaerwat from the input.yaml for single process test
  Using mam4::utils::extract_stateq_from_prognostics to copy data from prog to state_q.
  Refactor: move allocation of views from the tracer data struct interface to helper code. Combine multiple NC file registrations into a single function call: setup_tracer_data.
  Using one data structure to store beginning, end, and output data sets.
  Retrieve dgncur_a, dgncur_awet, wetdens, and qaerwat from FM
  1. Use the Scorpio interface to get altitude from vertical emission files. I was getting zeros or junk values when I employed the AtmInput class. 2. Add altitude_int as a member of the Altitude class.
  ...
…11'(PR #6568)

MOSART can be forced to crash if negative channel storage values are produced mainly due to round errors,
when the channel storage is completely depleted in a single time step.
The crashing was reported in several cases of E3SM fully coupled runs,
where the kinematic waving routing method was invoked by default.
Jon Wolfe was able to reproduce the crashing error in
a low-res run and print out detailed information to identify the cause.
In this fix, a numerical treatment has been implemented
in the kinematic wave routing method, subroutine Routing_KW(),
to ensure that any channel storage does not get completely
depleted in a single time step.
This treatment is physically reasonable as long as MOSART runs
at a sub-daily time step.
Jon Wolfe has tested this treatment in his low-res coupled run,
and it helped avoid crashing.

[non-BFB] for MOSART
- Recover ADIOS support
- Load libfabric/1.15.2.0 module
- Fix typos in configuration files
Update conditions for Southern Ocean ice and river runoff removal

This PR modifies the conditions for river and ice runoff removal in E3SM
cases. This changes the behavior of E3SM cases where data icebergs (DIB)
or ice-shelf melting (ISMF) were active and thus doesn't not affect
water cycle simulations.

River runoff:
  Previous behavior: never removed
  New behavior: removed when atmosphere is not active and ice shelf melt
                fluxes are on
Ice runoff:
  Previous behavior: removed when DIB is on and atmosphere is active
  New behavior: removed when DIB is on

[NML] when DIB or DISMF are active
[BFB] when DIB and DISMF are inactive (all WC configurations)
[non-BFB] when DIB or DISMF are active
Adding preliminary support for ADIOS output compression
and updating to SCORPIO v1.6.6 (which includes patch
to support the new I/O type to use for data compression)

Modifying CIME config and scripts to support writing
ADIOS output files with data compression. A new I/O
type is added to support writing output in ADIOS BP file
format with data compression.

However users would need to move to SCORPIO
master to write ADIOS output with data compression

Note: Writing ADIOS output with data compression is
now supported in SCORPIO master but is not
included in SCORPIO v1.6.6. A future release of
SCORPIO will include this feature

[BFB]
Set default config_check_ssh_consistency for oQU240wLI to false

Change the default config_check_ssh_consistency setting for oQU240wLI to
false, to avoid confusing warnings/errors from tests with this grid.
Otherwise we end up with log.ocean.err files complaining about:
   ERROR: Warning: Sea surface height outside of acceptable range (20m).
In general, mpaso sets config_check_ssh_consistency to false for grids
with ice shelf cavities, but this one was missed.

[NML] for configurations with oQU240wLI
[BFB]
This PR updates the machine and compiler files for Frontier (Nov. 2024), including the following:

* Recovery of ADIOS2 support
* Loading of the libfabric/1.15.2.0 module
* Correction of typos

[BFB] no baseline for Frontier yet.
…merge_2024_11_14

* upstream/master: (219 commits)
  Set default config_check_ssh_consistency for oQU240wLI to false
  Update machine and compiler files for Frontier
  Adding support for ADIOS builds with BLOSC
  fix formatting of changed md files
  update pam to match new p3 signature
  Add ghci-oci
  change the coupling period of MOSART to be half hour in land river two-way coupling to avoid time step mismatch
  increase time for wav tests and increase processor count for PEM test
  Updating to SCORPIO v1.6.6
  clean up seq_map_map line for Sw2o
  remove wav to ocean flux remapping
  remove WAV2OCN_FMAPNAME
  remove white space
  remove flux wave to ocean remapping (will be done in later PR)
  cleanup whitespace
  correction to langmuir mixing logic for LandIce Mask.
  trim down wav dev test suite
  add B case ICOS test to wav_developer test suite
  remove seq_map_map call for wave to ocn fluxes (no fluxes coupled yet)
  remove wav-atm, wav-ice coupling (for future PR)
  ...
Copy link
Contributor

mergify bot commented Nov 14, 2024

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce checks passing

Wonderful, this rule succeeded.

Make sure that checks are not failing on the PR, and reviewers approved

  • #approved-reviews-by >= 1
  • #changes-requested-reviews-by == 0
  • any of:
    • check-skipped={% raw %}gcc-openmp / ${{ matrix.build_type }}{% endraw %}
    • all of:
      • check-success="gcc-openmp / dbg"
      • check-success="gcc-openmp / fpe"
      • check-success="gcc-openmp / opt"
      • check-success="gcc-openmp / sp"
  • any of:
    • check-skipped={% raw %}gcc-cuda / ${{ matrix.build_type }}{% endraw %}
    • all of:
      • check-success="gcc-cuda / dbg"
      • check-success="gcc-cuda / opt"
      • check-success="gcc-cuda / sp"
  • any of:
    • check-skipped={% raw %}cpu-gcc / ${{ matrix.test.short_name }}{% endraw %}
    • all of:
      • check-success="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
      • check-success="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
      • check-success="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
      • check-success="cpu-gcc / SMS_D_Ln5.ne4pg2_oQU480.F2010-SCREAMv1-MPASSI.scream-mam4xx-all_mam4xx_procs"
  • any of:
    • check-skipped=cpu-gcc
    • check-success=cpu-gcc

@@ -200,6 +200,9 @@ option(SCREAM_MPI_ON_DEVICE "Whether to use device pointers for MPI calls" ON)
option(SCREAM_ENABLE_MAM "Whether to enable MAM aerosol support" ON)
set(SCREAM_SMALL_KERNELS ${DEFAULT_SMALL_KERNELS} CACHE STRING "Use small, non-monolothic kokkos kernels for ALL components that support them")
set(SCREAM_P3_SMALL_KERNELS ${SCREAM_SMALL_KERNELS} CACHE STRING "Use small, non-monolothic kokkos kernels for P3 only")
message("JGF DEFAUL_SMALL_KERNELS is ${DEFAULT_SMALL_KERNELS}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this meant to be just some debugging stuff?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, whoops! Let me remove that

bartgol
bartgol previously approved these changes Nov 14, 2024
Copy link
Contributor

@mahf708 mahf708 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A typo from my commit in the downstream merge

Context: Needed to format docs in the downstream to pacify the markdown linter ...

components/eamxx/docs/developer/kokkos_ekat.md Outdated Show resolved Hide resolved
Copy link

PR Preview Action v1.4.8
🚀 Deployed preview to https://E3SM-Project.github.io/scream/pr-preview/pr-3115/
on branch gh-pages at 2024-11-14 23:17 UTC

@jgfouca jgfouca added CI: automerge WARNING: Still in an experimental phase CI: workflow change approved Allow testing of PRs that alter a worfklow file labels Nov 14, 2024
@mergify mergify bot merged commit 746c90c into master Nov 15, 2024
14 checks passed
@mergify mergify bot deleted the jgfouca/upstream_merge_2024_11_14 branch November 15, 2024 02:51
@bartgol
Copy link
Contributor

bartgol commented Nov 15, 2024

Noting that the automerge feature of mergify worked (after not working for a long time). The merge commit is correctly formatted according to the mergify specs:

    Merge pull request #3115 from jgfouca/upstream_merge_2024_11_14
    
    Automatically merged using mergify
    PR title: Jgfouca/upstream merge 2024 11 14
    PR author: jgfouca
    PR labels: ['CI: workflow change approved', 'CI: automerge']

@mahf708
Copy link
Contributor

mahf708 commented Nov 15, 2024

@bartgol why did the tests not run tho?

@mahf708
Copy link
Contributor

mahf708 commented Nov 15, 2024

mergify(bot), good bot!

@bartgol
Copy link
Contributor

bartgol commented Nov 15, 2024

@bartgol why did the tests not run tho?

That is a very good question. The output of pre_process_pr shows "No relevant files touched by this PR", which is obviously not true, given the changes in components/eamxx. Perhaps I need to crank up the stuff I print in that action (e.g., the list of changed files, the patterns, the matches), so we can debug when stuff like this happens.

Ah. I executed the commands that the pre_process_pr job does, and indeed, there's no eamxx file listed:

(bartgol) lbertag@s1072983:[~]$ echo $changed_files
.github/workflows/e3sm-gh-ci-cime-tests.yml .github/workflows/e3sm-gh-ci-w-cime-tests.yml .github/workflows/e3sm-gh-md-linter.yml .github/workflows/e3sm-gh-pages.yml .github/workflows/e3sm-gh-tools-mkatmsrffile-test.yml .github/workflows/eamxx-gh-ci-standalone.yml .github/workflows/eamxx-gh-pages.yml .github/workflows/eamxx_default_files.yml CITATION.cff LICENSE README.md cime_config/allactive/config_compsets.xml cime_config/allactive/config_pesall.xml cime_config/config_grids.xml cime_config/machines/Depends.muller-cpu.gnu.cmake cime_config/machines/cmake_macros/amdclanggpu_frontier.cmake cime_config/machines/cmake_macros/crayclanggpu_frontier.cmake cime_config/machines/cmake_macros/gnu_chicoma-cpu.cmake cime_config/machines/cmake_macros/gnugpu_frontier.cmake cime_config/machines/cmake_macros/intel_dane.cmake cime_config/machines/cmake_macros/intel_ruby.cmake cime_config/machines/config_batch.xml cime_config/machines/config_machines.xml cime_config/testmods_dirs/io/force_adiosc/shell_commands cime_config/tests.py components/cmake/modules/FindPIO.cmake components/data_comps/datm/cime_config/config_component.xml components/data_comps/dlnd/src/dlnd_comp_mod.F90 components/data_comps/dlnd/src/lnd_comp_mct.F90 components/data_comps/docn/cime_config/config_component.xml

I wonder if the rest API is only returning the first X files. This list contains 30 files. Such a round number...

Edit: indeed, I changed

response=$(curl -s -H "Authorization: token <TOKEN>" "https://api.github.com/repos/E3SM-Project/scream/pulls/3115/files)"

to

response=$(curl -s -H "Authorization: token <TOKEN>" "https://api.github.com/repos/E3SM-Project/scream/pulls/3115/files?per_page=1000)"

and now I got 100 files. Still less than the actual number, so I think there's an upper limit. Which means we need to revisit the pre_process_pr logic... :/

See #3117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: automerge WARNING: Still in an experimental phase CI: workflow change approved Allow testing of PRs that alter a worfklow file
Projects
None yet
Development

Successfully merging this pull request may close these issues.