Skip to content

Adding feature tracking operator#2148

Open
Adam Gainford (A-Gainford) wants to merge 17 commits into
mainfrom
simple-track
Open

Adding feature tracking operator#2148
Adam Gainford (A-Gainford) wants to merge 17 commits into
mainfrom
simple-track

Conversation

@A-Gainford

@A-Gainford Adam Gainford (A-Gainford) commented May 19, 2026

Copy link
Copy Markdown

Adding feature-tracking via the Simple-Track repo (https://github.com/ParaChute-UK/simple-track), which is included as a new dependency. The feature.track operator is used to call Simple-Track, which returns three cubes per timestep: "feature_id", "feature_lifetime" and "feature_init". These can be filtered to select them for plotting with existing routines. Other data which is not currently used by CSET can be saved for further analysis. The new "feature" operator set will also be used to implement cell stats using Simple-Track with the tracking capabilities disabled (though the "feature" name may want to be changed).

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.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for this pull request, and sorry it took so long for me to get to it. I'm assuming that despite it being a draft PR it is ready for a review.

Overall it looks like a good set of changes that will nicely integrate simple-track into CSET. There are a few small issues highlighted in the comments below, but it is overall looking very good.

Please run the pre-commit checks over this code (pre-commit run --all-files) to fix a number of minor issues. These checks can be set to run on each commit by running pre-commit install.

Comment thread requirements/environment.yml
Comment thread src/CSET/operators/__init__.py Outdated
Comment thread src/CSET/operators/feature.py Outdated
Comment thread .gitignore Outdated
Comment thread src/CSET/operators/feature.py
Comment thread src/CSET/operators/feature.py
Comment thread tests/operators/test_feature.py Outdated
Comment thread tests/operators/test_feature.py Outdated
Comment thread src/CSET/recipes/example_recipes/example_feature_track.yaml Outdated
Comment thread src/CSET/recipes/example_recipes/example_feature_track.yaml
@jfrost-mo James Frost (jfrost-mo) marked this pull request as ready for review June 25, 2026 11:41
Adam Gainford (A-Gainford) and others added 10 commits June 30, 2026 15:22
Remove whitespace

Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Update copyright

Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the taking a look through the code, happy with the suggestions. I assume it's okay to accept changes on the lockfiles at the bottom of the PR page but I have held off for now

@Fraetor James Frost (Fraetor) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still need to review the feature file.

},
"feature_id": {
"cmap": "viridis",
"max": 4000,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this just a reasonable upper nound for the number of features tracked?

cmap, levels, norm = colorbar_map_levels(cube)

if "feature" in cube.long_name:
cmap.set_under("white")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Supposedly set_under is deprecated and we should be using with_extremes instead.
https://matplotlib.org/stable/api/_as_gen/matplotlib.colors.Colormap.html#matplotlib.colors.Colormap.set_under
https://matplotlib.org/stable/api/_as_gen/matplotlib.colors.Colormap.html#matplotlib.colors.Colormap.with_extremes

Given this can be set at colourway Creation maybe we should think about allowing this to be set in the colourbar file. Probably something for a different PR though.

Comment thread .gitignore
.nfs*

# MacOS temp files
.DS_Store No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.DS_Store
.DS_Store

As the pre-commit check notes, this file will need to end in a newline.

Fun fact: Strictly according to the POSIX specification a file that doesn't end in a new line isn't technically a text file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants