Repository navigation
Unified wrf/wrf-chem model_mod - #741
Open
hkershaw-brown wants to merge 80 commits into
Open
hkershaw-brown wants to merge 80 commits into
hkershaw-brown wants to merge 80 commits into
Conversation
New model_mod directory so development can happen concurrently on wrf and wrf_unified. Original pull request #683
will add back as needed
Collaborator
Author
|
For un reviewed atmos-chem forward operators see #743 |
The wrf-dart shell scripts are in the wrf/shell_scripts directory This runme_filter script is not used.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the WRF model_mod for both WRF and WRF CHEM by introducing a unified model_mod and updating corresponding documentation and configuration files. Key changes include:
- Addition of a build script (quickbuild.sh) in the wrf_unified directory.
- Extensive documentation updates in readme.rst describing the unified model and its configuration.
- A new model_mod.nml file with updated configuration and minor comment updates to observation modules.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| models/wrf_unified/work/quickbuild.sh | Introduces a build script for the new unified model |
| models/wrf_unified/readme.rst | Provides documentation updates for the unified WRF model interface |
| models/wrf_unified/model_mod.nml | Defines new namelist settings for the unified model_mod |
| assimilation_code/modules/observations/default_quantities_mod.f90 | Updates observation quantities comments with a new potential temperature entry |
| assimilation_code/modules/observations/atmosphere_quantities_mod.f90 | Updates observation quantities comments with a new potential temperature entry |
Comments suppressed due to low confidence (2)
assimilation_code/modules/observations/default_quantities_mod.f90:14
- [nitpick] Consider adding a brief comment that explains the rationale behind adding QTY_2M_POTENTIAL_TEMPERATURE to the observation quantities documentation.
! QTY_2M_POTENTIAL_TEMPERATURE
assimilation_code/modules/observations/atmosphere_quantities_mod.f90:107
- [nitpick] Consider adding documentation for the inclusion of QTY_2M_POTENTIAL_TEMPERATURE to clarify its intended use and context in the module.
! QTY_2M_POTENTIAL_TEMPERATURE
add wrf-chem to toc and list of available models
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟡 Changes recommended
Changes are blocked because review findings identify a projection increment bug in setup_map_projection that would break coordinate conversions and a critical broken adv_ens_command path in input.nml that would cause perfect_model_obs to fail unless corrected.
2 open issues
Review effort: Balanced
Comment on lines
+27
to
+29
| async = 0 | ||
| adv_ens_command = "../shell_scripts/advance_model.csh" | ||
|
|
hkershaw-brown
commented
Nov 9, 2026
| select case (which_vert) | ||
| case(VERTISLEVEL) | ||
| zloc(:) = lon_lat_vert(3) | ||
| level_below(:) = nint(zloc(:)) |
Collaborator
Author
There was a problem hiding this comment.
This is nearest level rather than level below. Can you have obs on fractional levels?
hkershaw-brown
commented
Nov 9, 2026
hkershaw-brown
commented
Nov 9, 2026
| call toGrid(xloc,i,dx,dxm) | ||
| call toGrid(yloc,j,dy,dym) | ||
|
|
||
| if ( .not. within_bounds_horizontal(i, j, grid_id, qty) ) then |
Collaborator
Author
There was a problem hiding this comment.
Q. where is your vertical bounds check?
hkershaw-brown
commented
Nov 9, 2026
hkershaw-brown
commented
Nov 9, 2026
|
|
||
|
|
||
| !------------------------------------------------------------------ | ||
| function bounds_check_vertical(ind, grid_id, state_id, var_id) |
Collaborator
Author
There was a problem hiding this comment.
This is not being called, see earlier comment about where is your vertical check.
hkershaw-brown
commented
Nov 9, 2026
hkershaw-brown
commented
Nov 9, 2026
hkershaw-brown
commented
Nov 9, 2026
hkershaw-brown
commented
Nov 9, 2026
hkershaw-brown
commented
Nov 9, 2026
QTY_GEOPOTENTIAL_HEIGHT is in the meterological domain
This was some experiments for bitwise differences with main. See #404 (comment) for notes on order of vertical and horizontal interpolation.
This is more for someone reading the code. The vortex qtys are not 'able_to_interpolate'
This is following wrf model_mod. I am not sure how strict to be with this in static_init_model_mod vs failing in column builds. There may be simplified runs where you do not care what is in the wrf state.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Refactored WRF model mod for WRF & WRF CHEM.
Namelist selectable QTYS, see #404 for details.
Created a wrf_unified model directory. This is to allow concurrent development on wrf and wrf_unified.
wrf_unified/model_mod is ~4500 less than the wrf/model_mod and this is a bit of a headache for keeping up-to-date with main (easy to miss changes in such differing code).
See pull #683 for original pull request
wrf-wrf-chem branch available at https://github.com/hkershaw-brown/DART/tree/wrf-wrf-chem
Fixes issue
Fixes #404
Partial #359 (WRF)
Partial #624 (WRF)
To do:
Follow fixes for
Using 'lat-lon' map projection in WRF, assigns Cassini projection in DART #811,
bug: pert_wrf_bc code introduces artifacts to perturbed WRF boundary files #807 from wrf model_mod - Adding new features to WRF scripting and tutorial #1030 forces hybrid_opt in gen_retroc_icbc.sh but not in model_mod.f90
hybrid coord#649 Add WRF hybrid vertical coordinate #650 Unified wrf/wrf-chem model_mod #741 (comment)
static data only in wrf mete domain - test.
sfc_elev_max_diff
Forcing QTY_TEMPERTAURE to be QTY_POTENTIAL_TEMPERATURE - this seems incorrect. Edit: updated the comment in bounds_check de9d3f8. The state variable is forced to be qty_potential_temperature. QTY_TEMPERATURE can be interpolated by model_interpolate.
cloudwater units micrometer vs m
model_interpolate when zloc is VERTISUNDEF
DART/models/wrf/model_mod.f90
Lines 1430 to 1436 in a32ab03
Types of changes
Documentation changes needed?
Tests
Please describe any tests you ran to verify your changes.
Bitwise tests during refactoring: https://github.com/hkershaw-brown/wrf-tests.git
Checklist for merging
Checklist for release
Testing Datasets