Skip to content

Various improvements to transect capability#1992

Merged
James Warner (jwarner8) merged 27 commits into
mainfrom
ml_transect
Apr 7, 2026
Merged

Various improvements to transect capability#1992
James Warner (jwarner8) merged 27 commits into
mainfrom
ml_transect

Conversation

@jwarner8
Copy link
Copy Markdown
Contributor

@jwarner8 James Warner (jwarner8) commented Mar 26, 2026

Broad collection of improvements to the transect capability in CSET, addressing the following issues/enhancement requests for pressure level transects;

Co-pilot was used to generate some of the code ideas used for the transect inset, which was further reviewed and adapted by JamesW.

Contribution checklist

Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.

  • Documentation has been updated to reflect change.
  • New code has tests, and affected old tests have been updated.
  • All tests and CI checks pass.
  • Ensured the pull request title is descriptive.
  • Ensure rose-suite.conf.example has been updated if new diagnostic added.
  • Conda lock files have been updated if dependencies have changed.
  • Attributed any Generative AI, such as GitHub Copilot, used in this PR.
  • Marked the PR as ready to review.

@jwarner8 James Warner (jwarner8) added the enhancement New feature or request label Mar 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 26, 2026

Coverage

@jwarner8
Copy link
Copy Markdown
Contributor Author

James Warner (jwarner8) commented Mar 26, 2026

Status 26/03
Modifications to transects have now been made
image

Started plumbing the aggregation side of things, need to debug why all models are being loaded for one model aggregation. It seems to load model 1 and 2, i.e. the parbaked recipe looks like

file_paths:

  • ~/cylc-run/cset_workflow/run11/share/cycle/*/data/1
  • ~/cylc-run/cset_workflow/run11/share/cycle/*/data/2

this leads to

Constraint doesn't produce single cube: ConstraintCombination(ConstraintCombination(ConstraintCombination(Constraint(), Constraint(cube_func=<function generate_cell_methods_constraint.<locals>.check_no_aggregation at 0x148eceefaae0>), <built-in function and_>), Constraint(name='x_wind'), <built-in function and_>), Constraint(coord_values={'pressure': <function generate_level_constraint.<locals>.<lambda> at 0x148eceefac40>}), <built-in function and_>)
0: x_wind / (m s-1) (time: 16; pressure: 28; latitude: 186; longitude: 142)
1: x_wind / (m s-1) (time: 16; pressure: 28; latitude: 186; longitude: 142)
2: x_wind / (m s-1) (time: 8; pressure: 13; latitude: 281; longitude: 321)
3: x_wind / (m s-1) (time: 8; pressure: 13; latitude: 281; longitude: 321)

A separate problem here is that cubes don't merge in a given model, but primary issue here is two models being loaded. Need to determine if other aggregation recipes do the same.

@jwarner8 James Warner (jwarner8) marked this pull request as draft March 30, 2026 13:41
@jwarner8
Copy link
Copy Markdown
Contributor Author

James Warner (jwarner8) commented Mar 30, 2026

Progress:

  • Looking at comparison between transect hour of day agg vs generic spatial agg, both have the same input path. Now examining differences in the loader.
  • Changing to loop over model in itertools seems to work, more generic issue of cubes not merging (also seen in pressure level aggregation) now being looked at.
  • Change operator from read_cube to read_cubes allows it to now work.

@jwarner8 James Warner (jwarner8) marked this pull request as ready for review March 30, 2026 15:44
@jwarner8
Copy link
Copy Markdown
Contributor Author

image Testing for other directions/transects looks OK

@jwarner8
Copy link
Copy Markdown
Contributor Author

Now ready for review, note their are no new tests, as its not clear how we would test a transect inset (other than trying it for different use cases and visually inspecting the output).

Copy link
Copy Markdown
Member

@jfrost-mo James Frost (jfrost-mo) left a comment

Choose a reason for hiding this comment

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

Not run myself, but the code change looks good. A couple minor suggestions but it is up to you if you take them.

Comment thread src/CSET/operators/plot.py Outdated
Comment thread src/CSET/operators/plot.py Outdated
Comment thread src/CSET/operators/plot.py
James Warner (jwarner8) and others added 3 commits April 7, 2026 11:15
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
@jwarner8
Copy link
Copy Markdown
Contributor Author

Not run myself, but the code change looks good. A couple minor suggestions but it is up to you if you take them.

Thanks for the review, these suggestions sound sensible and have now implemented, will go ahead and merge.

@jwarner8 James Warner (jwarner8) merged commit 1f85a8d into main Apr 7, 2026
8 checks passed
@jwarner8 James Warner (jwarner8) deleted the ml_transect branch April 7, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants