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

MODIS snow cover fraction assimilation #512

Merged

Conversation

gmao-rreichle
Copy link
Contributor

@gmao-rreichle gmao-rreichle commented Jan 10, 2022

Jul 10, 2023 update

Clean up and modifications to MODIS SCF assimilation:

  1. Modified the MODIS_SCF in LDASsa_DEFAULT_inputs_ensupd.nml to reflect recent changes to data location and switch from SCA to SCF to be more accurate.
  2. Included HTSNN1-3 and Albedo parameters in tile_bin2nc4.F90 to include necessary information in nc4 outputs needed for assimilation verification.
  3. Reorganized and cleaned up clsm_ensupd_read_obs.F90, including improved variable names and organization. Maintained the 1D restructuring of MODIS data.
  4. Modified clsm_ensup_upd_routines.F90 by removing most hard coded terms and renaming variables to be more specific to snow cover assimilation. There are 3 outstanding issues:
    * The apha, beta, and max_incr_swe terms are still hard coded within upd_routines. Rolf, Qing and I discussed where these should ultimately go, ideally in a location that can be changed at run time. Rolf will look into this.
    * Hard coded snow layer target thickness partially remains (Line 3519) The max top layer thickness can be imported from catch_constants.F90; however, the remaining values, which should be the distribution of thickness in layers 2 through N_snow in terms of fraction. These could be calculated within upd_routines as 1.0/real(N_snow-1, kind=4) following Stieglitz or could alternatively be exported along with the top layer thickness. In my mind, this decision depends on what relayer2 really needs. Does it simply need reasonable values (0.08 0.5 0.5) or should it be using the forecast values?
    * When there is snow in the forecast and the analysis, the code currently calculates the swe and heat content of each layer based on the swe ratio; however, there could be instances where the forecast snow layer is at 0 deg C and has liquid water within it. So, it may be more correct to calculate the heat content as a function of the forecast snow temperature and frozen fraction, which would look something like what I include in a comment on line 4568. I'd be interested in anyone's thoughts.
  5. Slight modifications to clsm_ensupd_enkf_update.F90 for clarity.

tagging @gmao-rreichle, @saraqzhang, @gmao-qliu, @amfox37

%%%%%%%
This PR continues where #434 left off.

Starting from Jongmin's branch (feature/jpark50/MODIS_snow_cover_fraction_assimilation), I removed three bad and redundant assignments in the MODIS SCA obs reader.
I also created an associated GEOSgcm_GridComp branch and edited the components.yaml file accordingly. The GEOSgcm_GridComp branch and its connection to the GEOSldas branch is not meant to be final and will need to be revisited.
These changes were implemented in Jongmin's sandbox but hadn't made it into his branch.

For reference, I copied Jongmin's sandbox and experiments to here:
/discover/nobackup/rreichle/Jongmin_SCA_DA/from_Jongmin/

cc: @lcandre2, @weiyuan-jiang, @gmao-qliu

jpark50 and others added 17 commits July 19, 2021 16:35
1) Updates regarding MODIS obs. reader
  -  Added the description of MODIS obs. reader at the subroutine
     (Table of longitude band is moved up)
  -  Changed the hdf-related function to read (vdata vs. SDS dataset)
     : Previous hdf-related functions are cleaned up
  -  Updated and checkced the QC procedure by comparing against Matlab result

2) Updates on subroutine cat_enkf_increment
  - Removing the cat_diagS at ths subroutine
    :Cat_diagS is also removed when the subroutine is called (e.g., ensupd_enkf_update.F90)
  - Computed the snow diagnostics to apply condition when to add/remove snow or no further action needed
: This commit has been made to update the error status located @ subroutine get_ind_obs()
: Basically, not significant change made on the code compared to the previous commit. All the print statements for debugging purposes are commented out
: Minor change made on the MODIS SCA reader(in clsm_ensupd_read_obs.F90) to correctly read the MODIS observation
: Things need to be done
  1) Check the compressed vector ('Observation') lat, long, and tilenum to see whether it correctly calculates or not
  2) Check the tile_grid properties
** Brief updates compared to commits made on 11/08/2021]
- No more errors related to the subroutine get_ind_obs()
- Checked the details of tilenum, Observation vector, and Obs_pred vector
  (meeting slide at 11/19/2021)
- Another floating error occured when calculating the snow heat content (located within the subroutine clsm_ensupd_upd_routine.F90)
  code line where showed error: TSN = min(met_force(n)%Tair-MAPL_TICE,0.0)
- I've tried to calculate the different between met_force%Tair -MAPL_TICE before entering the loop, and i defined it as deduction (array) and call   nth deduction value in the if loop. Still, it revealed a problem.
- For the review purpose, It would be great help if you can look close into the subroutine clsm_ensupd_upd_routine.F90 (from line 4076 ot 4261)
- fixed typos
- fixed indentation
- added/edited comments
- removed redundant initialization of cat_progn_incr
- removed redundant cat_progn_incr%wesn=0 etc in "do nothing" case of snow analysis
- undid partial (!) deletion of an old, commented-out subroutine
…_fraction_assimilation

land_assim: yes; pertubation:0; no mwRTM file
tclune
tclune previously approved these changes Jan 10, 2022
Copy link
Contributor

@tclune tclune left a comment

Choose a reason for hiding this comment

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

cmake changes ok

@weiyuan-jiang
Copy link
Contributor

weiyuan-jiang commented Jan 13, 2022

@jpark50 I just fixed your read_obs according to my understanding of your program ( I may be wrong though, please double check). But I believe the more serious error is the dimension of met_force that you passed into the subroutine cat_enkf_increments. The number N_catd and nTiles_ana are different if I understand correctly. You need to have a forcing that matches and re-arranges with nTiles_ana.
@gmao-rreichle

gmao-rreichle and others added 19 commits October 24, 2023 13:41
…n_assimilation (manually resolved conflict in clsm_ensupd_enkf_update.F90)
@gmao-rreichle
Copy link
Contributor Author

All GEOSldas nightly 0-diff tests passed after most recent commit.

@lcandre2
Copy link
Contributor

lcandre2 commented Mar 1, 2024

@gmao-rreichle, the newest iteration with Aqua and Terra runs cleanly.

@gmao-rreichle gmao-rreichle self-assigned this Mar 1, 2024
@gmao-rreichle gmao-rreichle marked this pull request as ready for review March 1, 2024 13:52
@mathomp4 mathomp4 added 0-diff trivial very, very obvious 0-diff change and removed 0-diff trivial very, very obvious 0-diff change labels Mar 1, 2024
@biljanaorescanin biljanaorescanin merged commit 970dbb8 into develop Mar 1, 2024
10 of 11 checks passed
@biljanaorescanin biljanaorescanin deleted the feature/rreichle/MODIS_snow_cover_fraction_assimilation branch March 1, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-diff enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement MODIS snow cover fraction assimilation
8 participants