GitHub Issue NOAA-EMC/GSI#118. EFSOI additions to src/gsi and src/enkf directories. #143
Conversation
|
PS - sorry for the bad PR title |
|
tagging @aerorahul @CatherineThomas-NOAA @MichaelLueken-NOAA (Liaofan not taggable) |
MichaelLueken
requested review from
MichaelLueken,
aerorahul,
CatherineThomas-NOAA and
dtkleist
src/enkf/params.f90
Outdated
| @@ -712,6 +761,23 @@ subroutine read_namelist() | |||
| corrlengthtr = corrlengthtr * 1.e3_r_single/rearth | |||
| corrlengthsh = corrlengthsh * 1.e3_r_single/rearth | |||
|
|
|||
| if(efsoi_cycling) then | |||
| letkf_flag = .false. | |||
There was a problem hiding this comment.
@AndrewEichmann-NOAA Do you understand what's happening here? Is LETKF turned off if you use EFSOI? I seem to remember Liaofan using LETKF in his tests.
There was a problem hiding this comment.
Apparently, I hadn't noticed that. The same lines are in Liao-Fan's fork. He might not have noticed either. I'll ask him.
There was a problem hiding this comment.
@CatherineThomas-NOAA @aerorahul
Apparently what was taken as evidence that LETKF has been running was actually the report of the namelist settings, but then after that these lines turn LETKF off. Grepping the stdout for "min/max/mean time to do letkf update" is more definitive. So apparently LETKF has not been run with EFSOI up to now. Removing these lines allows running LETKF and I have an EFSOI experiment running it transparently right now. However:
-
The initial osense files are identical between LETKF off and on, and contain the saved variable anal_obs. In the enkf code, anal_obs is reassigned only if efsoi_cycling=.true., and that's after an mpi send/receive with anal_obschunk, which does get updated in the preceding code. I presume that the mpi happens because EFSOI needs anal_obs, and the enkf code updates it (locally, as anal_obschunk) for its own use.
-
One difference between running EFSOI update with LETKF and not is that apparently the increments (atminc) in the ensemble members are set to zero when LETKF is turned off by this line. Which raises the question how we ever got any sensible results with v16. They are non-zero with the lines removed; ie it seems to be at least non-pathological with LETKF.
-
LETKF assigns anal_obs to the sensibly named hxens, which then goes on to be modified. The original anal_obs is left undisturbed, and output to the osense file.
-
The upshot is that the current code uses ensrf by accident, in a bad way that doesn't update anal_obs, which then gets saved to the osense file; whereas taking the lines out uses letkf, which doesn't update anal_obs because it's not expected to, does the same. That's why the osense files are identical between the two, and perhaps why the increments are zero with the accidentally run ensrf.
So I suspect what needs to be changed, aside from the elimination of these two lines, is, in the LETKF code, to send hxens (or something derived from it) back to anal_obs to be output into the osense file. If Ensrf is being broken as I suspect, is making sure it can work a concern? Three-day weekend, hello.
There was a problem hiding this comment.
@CatherineThomas-NOAA @aerorahul I think I understand some of what I have to do - what I need in the obsense file after the ensemble update is the product of the updated analysis perturbations in observation space and the inverse of R. In the enkf code if fso/efsoi_cycling is true after updating anal_obchunk, anal_obchunk gets gathered back into anal_ob, then later written to the osense file (this part is already in the master). In LETKF, it's not obvious to me how to get that, esp. since most of the final work seems to be in model space. Will keep looking.
There was a problem hiding this comment.
@CatherineThomas-NOAA @aerorahul From looking at the literature it looks like I can apply the same output from letkf_core to HXb to get HXa as is applied to Xb to get Xa, assuming H is linear. So approximately do to anal_ob_chunk what is already done to anal_chunk. Does that sound right?
There was a problem hiding this comment.
@CatherineThomas-NOAA I removed the lines that switched off LETKF when EFSOI is running. Now if letkf_flag and efsoi_cycling are true, the osense file will be saved with the observation space ensemble prior (when EFSOI wants the posterior). To run the serial filter and get the posterior out of that, lupd_obspace_serial must be true: this runs the serial filter, then the LETKF. This is fine unless efsoi_cycling is true, because that updates anal_ob with the posterior to write out to the osense file. The problem with that is that the LETKF then uses anal_ob as the prior. So I introduced a new array, anal_ob_post, which is declared in enkf_obsmod and allocated in the serial filter only if efsoi_cycling equals true. The posterior is copied into it from anal_ob_chunk to be written to the obsense, and anal_ob is left unchanged. It shouldn't affect anything else.
Also, I had to add gsi/constants.f90, which had some changes that I missed when making this PR.
AndrewEichmann-NOAA
force-pushed
the
EXP-efso_fv3_PR_src_enkf
branch
from
fc4b154
to
ea93dcd
Compare
|
@AndrewEichmann-NOAA It looks like something got messed up with this PR. It now says there are 203 files changed instead of 6. Maybe it's just not up to date with the master? |
|
@CatherineThomas-NOAA Should be fixed now - the 7th file is src/gsi/constants.f90, which was neglected in the original PR |
|
I kind of hate it tbh, but for now and for the application it seems like the best solution. |
|
@AndrewEichmann-NOAA - To reduce the number of commits from 12 to 1, and update your branch to the latest authoritative repo master, please use the following commands:
To merge the latest authoritative master into your PR branch:
Shorten commits to a single commit:
Commit and push:
Once these steps are complete, I will be able to send this work to the review committee. |
AndrewEichmann-NOAA
force-pushed
the
EXP-efso_fv3_PR_src_enkf
branch
from
7115e08
to
060c4ea
Compare
|
@MichaelLueken-NOAA I believe I have finished applying these steps |
|
@AndrewEichmann-NOAA Thanks, Andrew. Everything looks good. I will now send this work to the review committee. |
MichaelLueken
changed the title
|
There has been no feedback from the review committee, so I will now give final approval to these changes and merge them to the authoritative master. |
Modified following files in src/enkf for EFSOI implementation