Skip to content

Conversation

@mahf708
Copy link
Contributor

@mahf708 mahf708 commented Jul 13, 2025

Fixes #7506
Fixes #7510

[BFB]

@mahf708 mahf708 requested a review from bartgol July 13, 2025 19:37
@mahf708 mahf708 added bug fix PR EAMxx C++ based E3SM atmosphere model (aka SCREAM) labels Jul 13, 2025
@mahf708 mahf708 linked an issue Jul 13, 2025 that may be closed by this pull request
@mahf708 mahf708 added the BFB PR leaves answers BFB label Jul 13, 2025
@mahf708 mahf708 requested a review from meng630 July 14, 2025 23:06
Copy link
Contributor

@meng630 meng630 left a comment

Choose a reason for hiding this comment

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

Looks good and fixed #7265 (comment)

@meng630
Copy link
Contributor

meng630 commented Jul 15, 2025

@mahf708 Apologies that I got things mixed up. There's still an issue with remapping raw model simulations to target grids, say from ne256 run to output ne30 results. I can output mam4_external_forcing_vert_sum_dz_weighted at the original ne256 resolution, but once I add horiz_remap_file in the output.yaml, the model crashed to

0:  FAIL:
0: has_field(name, grid_name)
0: /lustre/orion/cli115/proj-shared/meng/E3SM/components/eamxx/src/share/field/field_manager.cpp:202
0: Error! Field mam4_external_forcing_vert_sum_dz_weighted on grid coarse_grid not found.

@mahf708
Copy link
Contributor Author

mahf708 commented Jul 15, 2025

Hmmm... @bartgol any idea?

@mahf708 Apologies that I got things mixed up. There's still an issue with remapping raw model simulations to target grids, say from ne256 run to output ne30 results. I can output mam4_external_forcing_vert_sum_dz_weighted at the original ne256 resolution, but once I add horiz_remap_file in the output.yaml, the model crashed to

0:  FAIL:
0: has_field(name, grid_name)
0: /lustre/orion/cli115/proj-shared/meng/E3SM/components/eamxx/src/share/field/field_manager.cpp:202
0: Error! Field mam4_external_forcing_vert_sum_dz_weighted on grid coarse_grid not found.

@bartgol
Copy link
Contributor

bartgol commented Jul 15, 2025

I'm not sure. But this is likely indep of resolution, so you can just run ne4, and use one of our ne4->ne2 map files, to get faster debug cycles. Then, either stuff a bunch of print out statements here/there or run through gdb and manually inspect stuff.

@mahf708 mahf708 changed the title EAMxx: fix diag naming for vert_contract EAMxx: fix diag naming for vert_contract and allow 1d wts Jul 16, 2025
@mahf708 mahf708 requested review from bartgol and meng630 July 16, 2025 02:54
@github-actions
Copy link

github-actions bot commented Jul 16, 2025

PR Preview Action v1.6.2

🚀 View preview at
https://E3SM-Project.github.io/E3SM/pr-preview/pr-7507/

Built to branch gh-pages at 2025-07-16 03:19 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@mahf708
Copy link
Contributor Author

mahf708 commented Jul 16, 2025

@meng630 I couldn't reproduce your error (see my test file below)

output.yaml

%YAML 1.1
---
filename_prefix: new_test
horiz_remap_file: /global/cfs/cdirs/e3sm/inputdata/atm/scream/maps/map_ne4pg2_to_ne2pg2_c20240902.nc
iotype: pnetcdf
averaging_type: average
max_snapshots_per_file: 1 # only one snapshot per file
fields:
  physics_pg2:
    field_names:
    - T_mid_vert_avg_dp_weighted
    - T_mid_vert_avg_dz_weighted
    - T_mid_vert_sum_dz_weighted
output_control:
  frequency: 1
  frequency_units: nsteps
restart:
  force_new_file: true

new_test.AVERAGE.nsteps_x1.0001-01-01-00000

netcdf new_test.AVERAGE.nsteps_x1.0001-01-01-00000 {
dimensions:
        time = UNLIMITED ; // (1 currently)
        dim2 = 2 ;
        ncol = 96 ;
        ilev = 73 ;
        lev = 72 ;
        lwband = 16 ;
        swband = 14 ;
variables:
        double time(time) ;
                time:units = "days since 0001-01-01 00:00:00" ;
                time:calendar = "noleap" ;
                time:bounds = "time_bnds" ;
        double time_bnds(time, dim2) ;
                time_bnds:units = "days since 0001-01-01 00:00:00" ;
                time_bnds:note = "right endpoint accumulation" ;
        float T_mid_vert_avg_dp_weighted(time, ncol) ;
                T_mid_vert_avg_dp_weighted:units = "K" ;
                T_mid_vert_avg_dp_weighted:_FillValue = 3.402824e+33f ;
                T_mid_vert_avg_dp_weighted:long_name = "MISSING" ;
                T_mid_vert_avg_dp_weighted:standard_name = "MISSING" ;
        float T_mid_vert_avg_dz_weighted(time, ncol) ;
                T_mid_vert_avg_dz_weighted:units = "K" ;
                T_mid_vert_avg_dz_weighted:_FillValue = 3.402824e+33f ;
                T_mid_vert_avg_dz_weighted:long_name = "MISSING" ;
                T_mid_vert_avg_dz_weighted:standard_name = "MISSING" ;
        float T_mid_vert_sum_dz_weighted(time, ncol) ;
                T_mid_vert_sum_dz_weighted:units = "K*m" ;
                T_mid_vert_sum_dz_weighted:_FillValue = 3.402824e+33f ;
                T_mid_vert_sum_dz_weighted:long_name = "MISSING" ;
                T_mid_vert_sum_dz_weighted:standard_name = "MISSING" ;
        float area(ncol) ;

@bartgol, you're not completely out of the woods yet ;):P

the hremap cannot handle rank-0 fields, trying T_mid_vert_sum_dz_weighted_horiz_avg in the above erros when hremapping, but is ok otherwise

0: idim>=0 && idim<m_rank
0: components/eamxx/src/share/field/field_layout.cpp:250
0: Error! Index out of bounds.

@bartgol
Copy link
Contributor

bartgol commented Jul 16, 2025

the hremap cannot handle rank-0 fields, trying T_mid_vert_sum_dz_weighted_horiz_avg in the above erros when hremapping, but is ok otherwise

0: idim>=0 && idim<m_rank
0: components/eamxx/src/share/field/field_layout.cpp:250
0: Error! Index out of bounds.

Do you have a stack trace for that? My guess is that it's the same issue that the abstract grid mods in PR #7515 are addressing. Can you just try to cherry pick the commit that touches abstract_grid.hpp from that PR, and see if the error persists?

Copy link
Contributor

@meng630 meng630 left a comment

Choose a reason for hiding this comment

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

Thanks @mahf708 for digging into this. It works well, and I believe it's ready to be merged.

@bartgol bartgol marked this pull request as draft July 17, 2025 16:12
@bartgol bartgol marked this pull request as ready for review July 17, 2025 16:12
bartgol added a commit that referenced this pull request Jul 17, 2025
@bartgol bartgol merged commit 1266c4c into master Jul 17, 2025
21 of 32 checks passed
@bartgol bartgol deleted the mahf708/eamxx/7506-eamxx-bug-in-name-of-vert_contract-diag branch July 17, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BFB PR leaves answers BFB bug fix PR EAMxx C++ based E3SM atmosphere model (aka SCREAM)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EAMxx: IO no longer works if outputting rank-0 field EAMxx: bug in name of vert_contract diag

4 participants