Conversation
Some of the content of this change has been produced with the assistance
of Anthropic Claude Opus 5 (Claude Code).
Adds two diagnostics:
thlev__pv Ertel potential vorticity on theta surfaces
dyn_trop__theta potential temperature on the PV = +/-2 surface
Both are vertical interpolations of fields the model already holds, so
no new science is introduced: gungho already computes Ertel PV on model
levels (W3) via potential_vorticity_diagnostic_alg, and theta_in_w3 is
already a derived field.
level_interp_mdi_kernel_mod interpolates a field onto surfaces of
constant value of a second field. It differs from the existing
pres_interp_kernel_mod in three ways:
* the bracketing level is the *highest* level at which the coordinate
field is at or below the target, so for a non-monotonic coordinate
such as |PV| the uppermost crossing is selected. This is what makes
dyn_trop__theta a dynamical tropopause rather than a boundary layer
artefact;
* columns which do not span the target are set to missing data rather
than extrapolated;
* cubic interpolation degrades to linear at both ends of the column.
Cubic interpolation is used for the theta surfaces, where the
coordinate is monotonic, and linear for the PV surface, taken against
|PV| = 2.0e-6 m2 s-1 K kg-1.
The kernel takes an array of target levels, which PSyclone cannot pass
(stfc/PSyclone#1312), so the PSy layer is hand written in
psykal_lite_phys_mod alongside invoke_pres_interp_kernel_type.
The theta_levels axis holds a single 320 K surface, following the TIGGE
standard. Both fields are added to the oper_nwp_gl output stream.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Some of the content of this change has been produced with the assistance of Anthropic Claude Opus 5 (Claude Code). Renames the two diagnostics into the processed__ namespace: thlev__pv -> processed__pv_on_theta_levs dyn_trop__theta -> processed__theta_on_pv2 The XIOS output names and the local field names in pv_surface_diags_alg_mod follow suit. The theta_levels axis now carries the 475, 425 and 315 K surfaces rather than a single 320 K surface. The axis values are listed in the order requested; the kernel treats each target level independently, so the ordering affects only the layout of the output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds processed__u50m and processed__v50m, the eastward and northward wind components interpolated onto a height surface 50 m above the ground. Both reuse the existing u_in_w3 and v_in_w3 derived fields and the level_interp_mdi kernel, so only the vertical coordinate is new: height_above_surface_kernel_mod subtracts the lowest Wtheta level, this being the orography, from the W3 heights above mean sea level. Interpolation is linear. Some of the content of this change has been produced with the assistance of Anthropic Claude Opus 5 (Claude Code).
github-actions
Bot
added
the
cla-modified
Andrew Coughtrie (andrewcoughtrie)
left a comment
Andrew Coughtrie (andrewcoughtrie)
left a comment
There was a problem hiding this comment.
Code Owner Review
I'm happy with the changes to the files I am listed as code owner of.
mo-cjsmith
left a comment
mo-cjsmith
left a comment
There was a problem hiding this comment.
Replicates UM diagnostics. Code and results look fine.
Thanks Chris - have addressed your comments in 22c6ac1. Over to Steve Mullerworth (@stevemullerworth) for code review |
Steve Mullerworth (stevemullerworth)
left a comment
Steve Mullerworth (stevemullerworth)
left a comment
There was a problem hiding this comment.
Looks fine. The only request is to be explicit about the use, I assume, of Claude under the umbrella of Met Office Enterprise Copilot.
Actually, it's not, it is just a Met Office Claude installation which is being used to accelerate the backlog of diagnostics required for PS49. Its use has been approved by Yash and the NC domain board, and Yash agreed to the wording of the attribution (and has #679 to update the working practices). |
|
As I am not party to these discussions and not knowledgeable about different AI deployments, I have asked Yaswant Pradhan (@yaswant) to provide a review on this aspect of the change. It seems to me (and to Copilot) that currently Enterprise Github Copilot is the only permitted generation tool according to published guidance. Also, as yet, the current branch and ticket does not meet the standard of the proposed changes to the working practice in #679 as those changes have additional questions and require attribution within commit messages. |
That's fine, although note that #664 which used similar tools has already been committed. I believe everything listed in #679 has been done, i.e. every commit by Claude has that mentioned in the commit message (and as Erica pointed out when reviewing #664, somewhat annoyingly the Claude commits have all come up as unverified, which is something I can look at fixing in future branches). |
Hi iboutle based on the proposed policy, free-tier contributions are not permitted in simulation system repositories. Even with paid models, it is still a bit complicated. We need some clarity around this, I'll discuss with you offline. |
Hi Yaswant Pradhan (@yaswant) - it's not free-tier. I think you need to discuss with BShipway and James Penn (@jamesp) (and probably Kier), as my understanding that use of this tool by myself (and Byron and Zubair) had already been approved. |
|
Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR. Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up. |
6 participants
PR Summary
Sci/Tech Reviewer: mo-cjsmith
Code Reviewer: Steve Mullerworth (@stevemullerworth)
Diagnostics requested by R2O for downstream users as part of PS49
N.B. The UM diagnostics 212-213 are actually incorrect - they are 50m above the lowest rho level (which typically lies at 10m in the global model). We have implemented them correctly here as 50m above the surface.
Some example output plots:
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - pv_theta_diags/run1
Suite Information
Task Information
✅ succeeded tasks - 1213
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review