Skip to content

Refine realization coordinate handling when clustering#2361

Open
gavinevans wants to merge 5 commits into
metoppv:masterfrom
gavinevans:mobt_783_ensure_realization_is_dimension
Open

Refine realization coordinate handling when clustering#2361
gavinevans wants to merge 5 commits into
metoppv:masterfrom
gavinevans:mobt_783_ensure_realization_is_dimension

Conversation

@gavinevans
Copy link
Copy Markdown
Contributor

Related to https://github.com/metoppv/mo-blue-team/issues/783

Description
This PR makes edits to the realization clustering code, so that the realization coordinate is handled correctly. The changes are:

  1. the realization coordinate within the primary cubes is renumbered. This renumbering is required if different primary cubes have different realization numbers, which would otherwise prevent these cubes being merged. I've made this optional, in case the automatic renumbering isn't appropriate, which could be the case, depending on the meaning of realization numbers. I was also conscious that the realization number of the primary cube is used for the deterministic realization selection implemented in Mobt 1064 deterministic realization selection #2337.
  2. the promotion of the realization coordinate from an auxiliary coordinate to a dimension coordinate. This seems to be required when a cube is indexed using non-monotonic index (e.g. [3, 1, 2]). When the realization dimension is indexed in this way, it results in the realization coordinate being demoted to being an auxiliary coordinate with the cube having an anonymous dimension coordinate with which the the demoted realization coordinate is associated. Promoting the realization coordinate reverts the unintended demotion.

Testing:

  • Ran tests and they passed OK
  • Added new tests for the new feature(s)

…nsion coordinate regardless of how the cubes are indexed.
…there is some meaning to these numbers, which means automatic renumbering isn't appropriate.
@maxwhitemet maxwhitemet self-requested a review April 20, 2026 09:16
@maxwhitemet maxwhitemet self-assigned this Apr 20, 2026
Copy link
Copy Markdown
Contributor

@maxwhitemet maxwhitemet left a comment

Choose a reason for hiding this comment

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

Thanks @gavinevans. I have suggested some minor changes

Comment thread improver/cli/realization_cluster_and_match.py
Comment thread improver/clustering/realization_clustering.py
Comment thread improver/clustering/realization_clustering.py Outdated
Comment thread improver_tests/clustering/test_realization_clustering.py Outdated
random_state=42,
).process(cubes)

assert "realization" in [coord.name() for coord in result.dim_coords]
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.

I recently came across this pattern

Suggested change
assert "realization" in [coord.name() for coord in result.dim_coords]
assert result.coords("realization", dim_coords=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Amended.

n_clusters = len(result.coord("realization").points)
assert n_clusters == 3
assert len(result.coord_dims("realization")) == 1
assert "realization" in [coord.name() for coord in result.dim_coords]
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.

assert result.coords("realization", dim_coords=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Amended.

# Should have 3 clusters
assert len(result.coord("realization").points) == 3
assert len(result.coord_dims("realization")) == 1
assert "realization" in [coord.name() for coord in result.dim_coords]
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
assert "realization" in [coord.name() for coord in result.dim_coords]
assert result.coords("realization", dim_coords=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Amended.

Comment thread improver_tests/clustering/test_realization_clustering.py Outdated
…ealization_is_dimension

* upstream/master:
  Option to ignore grid dimensions hash check on SpotExtraction (metoppv#2360)
  Apply realization clustering result to any lead time (metoppv#2340)
@gavinevans gavinevans force-pushed the mobt_783_ensure_realization_is_dimension branch from 47dc7f2 to 39e1675 Compare April 20, 2026 16:25
@gavinevans gavinevans assigned maxwhitemet and unassigned gavinevans Apr 20, 2026
@maxwhitemet
Copy link
Copy Markdown
Contributor

Thanks for the changes @gavinevans. Approved 👍

@maxwhitemet maxwhitemet removed their assignment Apr 21, 2026
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.

2 participants