Skip to content

Conversation

@mos3r3n
Copy link
Collaborator

@mos3r3n mos3r3n commented Apr 20, 2025

Description

Previously, the ensemble mean and variance were calculated using an external executable. This PR uses the mpasjedi_ens_mean_variance.x within MPAS_JEDI to do it.

Issue closed

Closes #(if applicable)

Tests completed

Tier 1:

  • 3dvar_OIE120km_WarmStart
  • 3denvar_OIE120km_IAU_WarmStart
  • 3dvar_OIE120km_ColdStart
  • 3dvar_O30kmIE60km_ColdStart
  • 3denvar_O30kmIE60km_WarmStart
  • eda_OIE120km_WarmStart
  • getkf_OIE120km_WarmStart
  • letkf_OIE120km_WarmStart
  • ForecastFromGFSAnalysesMPT

Tier 2 (optional):

  • GenerateGFSAnalyses
  • GenerateObs

rain_water \
snow_water \
)
set MPASHydroStateVariables = (${MPASHydroIncrementVariables} cldfrac)
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I ran test/testinput/eda_OIE120km_WarmStart.yaml the HofX steps all failed with
mpasjedi_vc_model2geovars::changevar: cldfrac must be added to the state variables in order to populate the var_cldfrac geovar with the MPAS diagnostic cloud fraction

I reverted the change to config/mpas/variables.csh and the test case passed.
I also reran test/testinput/letkf_OIE120km_WarmStart.yaml with the reverted variables.csh and it passed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I get the same for several tests on the HofX step.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used it to also calculate the ensemble mean of cldfrac. Without specified in state variables, the mean of cloud_area_fraction_in_atmosphere_layer could not be calculated. Maybe we need to add both? I am not sure but will try it later.

Copy link
Collaborator

@byoung-joo byoung-joo Apr 22, 2025

Choose a reason for hiding this comment

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

I am a bit confused. The model variable name cldfrac has not changed as a generic name yet (i.e., cloud_area_fraction_in_atmosphere_layer). It is also the same for effective_radius_of_xxxx variables. All these variable should go through the "variable change". I think this PR would not work with develop branch of mpas-jedi. @mos3r3n, I wonder if this PR is compatible with some of your feature branch, not with develop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is from the develop branch. I thought those variables were general variables already. I will make a commit to correct them back.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, BJ!
I modified them to cldfrac, re_cloud, re_ice, and re_snow. Now it works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, I tested and it HoxF works now.

Comment on lines +17 to +19
re_cloud \
re_snow \
re_ice \
Copy link
Collaborator

@byoung-joo byoung-joo Apr 25, 2025

Choose a reason for hiding this comment

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

@mos3r3n , I'd like to confirm that the results are the same (for example, ensMeanVariance) even if we remove these three variables, re_cloud, re_snow, and re_ice.

To clarify the procedure, the list of state variables should come from yaml config. This is not the case in current mpas-jedi's develop branch. See for example https://github.com/JCSDA-internal/mpas-jedi/blob/develop/src/mpasjedi/State/mpas_state_interface_mod.F90#L70-L94 .
This was discussed with @mos3r3n and @jim-p-w, separately before.

The usecase of mpas-jedi is getting more complex, so it would be nice to make this part clear in near future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If these variables are not included, their mean will not be calculated. For all-sky HofX of the ensemble mean, these variables should be available.
I haven't tested the DA applications. Maybe we need to tested via 1-week cycling test @jim-p-w .

Copy link
Collaborator

@byoung-joo byoung-joo Apr 29, 2025

Choose a reason for hiding this comment

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

I realized that the arithmetic operations only work with vars_ci, which comes from the yaml directly. So this is a necessary change for mpasjedi_ens_mean_variance.x.

air_potential_temperature \
dry_air_density \
u \
w \
Copy link
Collaborator

@byoung-joo byoung-joo Apr 25, 2025

Choose a reason for hiding this comment

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

Is this a necessary change ?

Copy link
Collaborator Author

@mos3r3n mos3r3n Apr 25, 2025

Choose a reason for hiding this comment

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

I'm not quite sure whether the w variable is a necessity to initialize the model.
If in the future we assimilate radial velocity data, this variable should be added to the state variables.

Comment on lines 47 to 48
air_temperature_at_2m \
water_vapor_mixing_ratio_wrt_moist_air_at_2m \
Copy link
Collaborator

@byoung-joo byoung-joo Apr 25, 2025

Choose a reason for hiding this comment

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

Is this a necessary change ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think for future surface assimilation purpose, these two variables could be added.

@byoung-joo
Copy link
Collaborator

In my comments above, I am trying to keep the necessary changes that are relevant to this PR in general.

Additional features can be done in separate PRs. This will also help us keep track of changes in this repository.

@mos3r3n
Copy link
Collaborator Author

mos3r3n commented Apr 28, 2025

In my comments above, I am trying to keep the necessary changes that are relevant to this PR in general.

Additional features can be done in separate PRs. This will also help us keep track of changes in this repository.

Thanks @byoung-joo! I agree that T2 and Q2 are not used in DA or prognostic variables of MPAS, so I will not add them in this PR. The w wind is a prognostic variable of MPAS, so I will keep it in this PR.

@byoung-joo
Copy link
Collaborator

The w wind is a prognostic variable of MPAS, so I will keep it in this PR.

Sound good to me. This application seems to be unique. It reads the ensemble using "background" stream and writes out using "background" stream. w is already included in our stream_list.atmosphere.background.

I think that adding w to state variabe list would not affect the existing variational application.

@byoung-joo
Copy link
Collaborator

I confirmed that adding w to StandardAnalysisVariables does not affect the existing Variational application.
I have not tested the effect of adding re_cloud, re_ice, and re_snow to MPASHydroStateVariables, but in my understanding this will not affect the existing Variational application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants