Skip to content

Add ilev as a default output in all EAMxx output streams #3091

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 12, 2024
Merged

Add ilev as a default output in all EAMxx output streams #3091

merged 4 commits into from
Nov 12, 2024

Conversation

AaronDonahue
Copy link
Contributor

@AaronDonahue AaronDonahue commented Nov 4, 2024 •
edited by mahf708
Loading

This commit follows the example for the geometry data for 'lev' and adds 'ilev'. The variable 'ilev' will now be a default variable in all EAMxx output similar to what is currently done with lat/lon, hyam/hybm, hyai/hybi and lev.

Fixes #3022

@AaronDonahue
Add ilev as a default output in all EAMxx output streams
f72cc87 
This commit follows the example for the geometry data for 'lev'
and adds 'ilev'.  The variable 'ilev' will now be a default variable
in all EAMxx output similar to what is currently done with lat/lon,
hyam/hybm, hyai/hybi and lev.

Addresses #3022
@AaronDonahue AaronDonahue added New Variables New variables added, B4B in old variables I/O labels Nov 4, 2024
@AaronDonahue AaronDonahue requested a review from bartgol November 4, 2024 20:07
@mahf708
Copy link
Contributor

mahf708 commented Nov 5, 2024

@bartgol @AaronDonahue how about we extract the logic for this type of op such that it can be generalized (and tidied up)? Something akin to add_dim_info(details) where the details would have the final (not intermediate) naming, values, etc. for everything to be added

@bartgol if that seems feasible, I can probably impl ...

@bartgol
Copy link
Contributor

bartgol commented Nov 7, 2024

@bartgol @AaronDonahue how about we extract the logic for this type of op such that it can be generalized (and tidied up)? Something akin to add_dim_info(details) where the details would have the final (not intermediate) naming, values, etc. for everything to be added

@bartgol if that seems feasible, I can probably impl ...

Where would you add this? Inside I/O? Or as a method of the grid? Generally speaking, I think it should be feasible. It could be as simple as offering a small method in the grid class, which internally simply calls "set_geo_data"...

@AaronDonahue
Copy link
Contributor Author

AaronDonahue commented Nov 8, 2024 •
edited
Loading

I'm afraid of this PR getting lost in limbo. Would it make sense to merge the PR as is now and clean up once we have the impl discussed above? @bartgol @mahf708

@mahf708
Copy link
Contributor

mahf708 commented Nov 9, 2024 •
edited
Loading

I'm afraid of this PR getting lost in limbo. Would it make sense to merge the PR as is now and clean up once we have the impl discussed above? @bartgol @mahf708

Yep! (Note it seems you have real fails going on the tests though, so these need to be fixed)

@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 11, 2024 •
edited
Loading

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-success="gcc-openmp / dbg"
    • check-skipped="gcc-openmp / dbg"
  • any of:
    • check-success="gcc-openmp / sp"
    • check-skipped="gcc-openmp / sp"
  • any of:
    • check-success="gcc-openmp / fpe"
    • check-skipped="gcc-openmp / fpe"
  • any of:
    • check-success="gcc-openmp / opt"
    • check-skipped="gcc-openmp / opt"
  • any of:
    • check-success="gcc-cuda / dbg"
    • check-skipped="gcc-cuda / dbg"
  • any of:
    • check-success="gcc-cuda / sp"
    • check-skipped="gcc-cuda / sp"
  • any of:
    • check-success="gcc-cuda / opt"
    • check-skipped="gcc-cuda / opt"
  • any of:
    • check-success="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
    • check-skipped="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
  • any of:
    • check-success="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
    • check-skipped="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
  • any of:
    • check-success="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
    • check-skipped="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
  • any of:
    • check-success="cpu-gcc / SMS_D_Ln5.ne4pg2_oQU480.F2010-SCREAMv1-MPASSI.scream-mam4xx-all_mam4xx_procs"
    • check-skipped="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

@AaronDonahue AaronDonahue merged commit aca29bc into master Nov 12, 2024
18 checks passed
@AaronDonahue AaronDonahue deleted the aarondonahue/io/add_ilev_to_output branch November 12, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartgol bartgol bartgol approved these changes

@mahf708 mahf708 mahf708 approved these changes

Assignees
No one assigned
Labels
I/O New Variables New variables added, B4B in old variables
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ilev not being written to output files
3 participants
@AaronDonahue @mahf708 @bartgol