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

(1) add ldas anal increments to fcst script #147

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

saraqzhang
Copy link
Contributor

    (1) add ldas analysis increments to g5fcst script that is generated from fvsetup 
    (2) fix syntax errors in the current develop fvsetup 

        (2) fix syntax errors in fvsetup
	modified:   src/Applications/GEOSdas_App/fvsetup
@saraqzhang saraqzhang requested a review from a team as a code owner December 16, 2021 03:20
@gmao-rreichle gmao-rreichle added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs labels Dec 16, 2021
@gmao-rreichle gmao-rreichle linked an issue Dec 16, 2021 that may be closed by this pull request
@gmao-rreichle gmao-rreichle added 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) bug fix and removed Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs 0 diff The changes in this pull request have verified to be zero-diff with the target branch. labels Dec 16, 2021
@gmao-rreichle
Copy link
Contributor

@rtodling , @gmao-jstassi : Here's a quick summary of this PR:

  1. The PR fixes a bug in fvsetup that was introduced recently in an unrelated PR (nothing to do with the land). You may already have fixed this bug separately.

  2. The PR further makes a trivially 0-diff change that applies increments from the LDAS to forecasts during the first few hours, which are just a re-run of the Corrector segment (similar to applying IAUs during the initial period). This change applies only when the ADAS is coupled to the LDAS (hence trivially 0-diff). According to @saraqzhang, this change needs to be made in fvsetup, which is not ideal because fvsetup is already bulky. Let us know if you can think of a different way to implement this.

  3. Besides the verification in the coupled ADAS-LDAS system, we did not test the PR because it seems to be trivially 0-diff. Let us know if you need us to do additional testing.

There's no rush in merging this PR, but it would be good not to keep it in limbo for very long to avoid having to maintain it as "develop" evolves. Let us know if you have any questions, thanks, - Rolf

Copy link
Collaborator

@rtodling rtodling left a comment

Choose a reason for hiding this comment

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

ok

@rtodling rtodling merged commit 12b10bc into develop Dec 17, 2021
@gmao-rreichle gmao-rreichle deleted the feature/saraqzhang/ldasIncr_replay branch December 17, 2021 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add ldas analysis increments in fcst
3 participants