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

Update varobs and cx writers for visibility #221

Merged
merged 18 commits into from
Sep 6, 2024

Conversation

ReubenHill
Copy link
Contributor

@ReubenHill ReubenHill commented Aug 15, 2024

The changes made in this PR are necessary for the assimilation of visibility by VAR. There are three key changes:

  1. The surface namelist has been updated to include model fields for visibility, surface total water (qt2) and aerosol (specifically with stash code 90, called "Total Aerosol (for Vis)").
  2. The cx writer has been updated to allow surface total water and aerosol to be output. These are used by VAR, alongside other model variables, to calculate background visibility.
  3. The varobs writer will now write ObsValue/horizonalVisibility values to VarField_logvis.

Updating the cx writer for outputting aerosol was non-trivial:

  1. There are multiple aerosol stash codes - 90 is the correct one to use for the calculation of background visibility in VAR.
  2. The field is defined over all 70 levels, but OPS only outputs the level closest to the surface: see the comments in src/opsinputs/opsinputs_fill_mod.F90 for details. This required special case code.

@adammaycock this was the code you were helping me to debug last month: the issue was an incorrect stash code causing the necessary select case to be skipped over.

@ss421 I have now confirmed that I need the following for VAR to run, so please don't exclude them from FieldNameCompare.xlsx:

  • qt2 - surface total water - stash code 3255
  • aerosol - Total Aerosol (for Vis) - stash code 90

NOTE: the ukv surface KGO will need to be remade once this is merged.

Copy link
Collaborator

@ctgh ctgh left a comment

Choose a reason for hiding this comment

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

Very nice! Does this require any modifications to sith KGO output?

src/opsinputs/opsinputs_fill_mod.F90 Outdated Show resolved Hide resolved
@ReubenHill
Copy link
Contributor Author

Very nice! Does this require any modifications to sith KGO output?

I don't think so: don't KGOs only use the NetCDF output from the jopa background task? This ought not to effect that.

@ctgh
Copy link
Collaborator

ctgh commented Aug 16, 2024

I believe they check ODB, Varobs and CX output. It might be worth running it just to be on the safe side.

@ReubenHill
Copy link
Contributor Author

ReubenHill commented Aug 16, 2024

I believe they check ODB, Varobs and CX output. It might be worth running it just to be on the safe side.

Yes you are correct, the kgos will need to be remade. Unless it's very straightforward, it's probably only worth doing once the jjdocs PR is ready since that will also need new kgos.

Copy link
Collaborator

@ctgh ctgh left a comment

Choose a reason for hiding this comment

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

Thanks for making the change. The ctest failure is fixed in #222

Copy link
Collaborator

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

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

Ran OK in my environment. No failures detected for mo-bundle main build + ctest, and sith/malak kgo runs (apart from what's expected).

@ReubenHill
Copy link
Contributor Author

ReubenHill commented Aug 16, 2024 via email

@matthewrmshin
Copy link
Collaborator

matthewrmshin commented Aug 16, 2024 via email

This reverts commit b4efa39. Either the
syntax is wrong or this is not allowed.
@matthewrmshin
Copy link
Collaborator

This change appears to require KGO changes for various Surface.* files for sith kgo_glu and kgo_ukv as well as malak kgo_glu_jopa_jada and kgo_glu_jopa_var - so not just the UKV ones. Is this expected?

@ReubenHill
Copy link
Contributor Author

This change appears to require KGO changes for various Surface.* files for sith kgo_glu and kgo_ukv as well as malak kgo_glu_jopa_jada and kgo_glu_jopa_var - so not just the UKV ones. Is this expected?

The varobs writer now outputs visibility - since these are in the list of observed variables for both global and ukv yamls, I therefore expect both sets of varobs files to change. The changes here should not effect global surface cx files, though I can imagine that a change to a varobs file might somehow necessitate a change to a cx file. What exact differences do you see?

@matthewrmshin
Copy link
Collaborator

Differences reported for these files:

malak:

  • kgo_glu_jopa_jada:
    • glu_jopa_varobs/Surface.varobs
  • kgo_glu_jopa_var:
    • glu_jopa_varobs/Surface.varobs

sith:

  • kgo_glu:
    • glu_jopa_varobs/Surface.varobs
    • glu_jopa_varobs_screen/Surface.varobs
  • kgo_ukv:
    • ukv_jopa_var_cx/Surface.cx
    • ukv_jopa_varobs/Surface.varobs

See outputs of these suites of mine: http://fcm1/cylc-review/suites?user=frsn&names=*oi221-kgo_glu*+*oi221-kgo_ukv

@ReubenHill
Copy link
Contributor Author

Differences reported for these files:

malak:

  • kgo_glu_jopa_jada:

    • glu_jopa_varobs/Surface.varobs
  • kgo_glu_jopa_var:

    • glu_jopa_varobs/Surface.varobs

sith:

  • kgo_glu:

    • glu_jopa_varobs/Surface.varobs
    • glu_jopa_varobs_screen/Surface.varobs
  • kgo_ukv:

    • ukv_jopa_var_cx/Surface.cx
    • ukv_jopa_varobs/Surface.varobs

See outputs of these suites of mine: http://fcm1/cylc-review/suites?user=frsn&names=*oi221-kgo_glu*+*oi221-kgo_ukv

These are as expected. Whilst the global varobs files now contain visibility (as the OPS ones always have), the global JOPA VAR task job.stats outputs before and after are identical - i.e. the presence of visibility in the global surface JOPA output has no impact on VAR. So please go ahead and recreate the KGOs.

@matthewrmshin matthewrmshin added waiting for kgo update waiting for kgo update ready to merge and removed do not merge waiting for kgo update waiting for kgo update labels Sep 5, 2024
@matthewrmshin matthewrmshin merged commit 2d6b182 into develop Sep 6, 2024
7 of 9 checks passed
@matthewrmshin matthewrmshin deleted the feature/ukv_surface_add_vis branch September 6, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants