Copilot makes a decomp - #161
Open
Andrew Clark (arjclark) wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a new ants.decomposition.simple_split_and_process framework intended as a simpler, serial, in-memory alternative to the existing decompose workflow, along with a comprehensive test battery validating split vs non-split invariants across multiple domains and regridding modes.
Changes:
- Added
simple_split_and_processplus_ssp_*helper functions for splitting, source-region extraction (including wrap-around and mixed longitude conventions), and piece concatenation/dtype handling. - Added a new, parametrized test module covering unary ops, binary (linear + area-weighted) ops, mixed longitude conventions, and a dtype-concatenation regression.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| lib/ants/decomposition.py | Adds the new simple_split_and_process decomposition path and its private _ssp_* helpers (splitting, extraction, concatenation). |
| lib/ants/tests/decomposition/test_simple_split_and_process.py | New test suite validating correctness invariants across split configurations, domains, and regridding methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1131
to
+1142
| # With zero splits in both dimensions, apply the operation directly to the | ||
| # full datasets without any decomposition overhead. | ||
| if number_of_x_splits == 0 and number_of_y_splits == 0: | ||
| if target is not None: | ||
| return operation(source, target) | ||
| else: | ||
| return operation(source) | ||
|
|
||
| # Guess coordinate bounds on the full cubes before splitting. This must | ||
| # be done here (on the intact cubes) rather than inside the splitting | ||
| # loop: guessing bounds on a sub-region can produce slightly different | ||
| # spacing for circular grids, making the pieces inconsistent. |
Comment on lines
+721
to
+726
| x_coordinate, y_coordinate = ants.utils.cube.horizontal_grid(cube) | ||
| # coord_dims returns a tuple; for rectilinear grids each coordinate maps to | ||
| # exactly one dimension index. | ||
| x_dimension_index = cube.coord_dims(x_coordinate)[0] | ||
| y_dimension_index = cube.coord_dims(y_coordinate)[0] | ||
|
|
Comment on lines
+830
to
+833
| source_x_coordinate, source_y_coordinate = ants.utils.cube.horizontal_grid(source) | ||
| target_x_coordinate, target_y_coordinate = ants.utils.cube.horizontal_grid( | ||
| target_piece | ||
| ) |
Comment on lines
+341
to
+342
| assert_decomposed_result_matches_reference(decomposed, reference) | ||
| def test_global_source_australia_target( |
Comment on lines
+357
to
+358
| assert_decomposed_result_matches_reference(decomposed, reference) | ||
| def test_global_source_new_zealand_target( |
Comment on lines
+101
to
+103
| The cube covers the full globe at approximately 5-degree resolution | ||
| (9 rows x 18 columns), finer than the 10-degree source but still small | ||
| enough for fast tests. |
Comment on lines
+123
to
+124
| The domain (-45 to -10 N, 110 to 155 E) lies entirely in the southern | ||
| hemisphere, well away from the wrap-around boundaries. Both -180:180 and |
Comment on lines
+134
to
+135
| The domain (-48 to -33 N, 165 to 180 E) lies near the International | ||
| Dateline at 180 degrees east. This tests that extraction near the eastern |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
To be completed prior to review request and updated as required during the review process.
If the answer to an item on the list is not applicable, feel free to replace the checkbox with 'N/A' to give extra clarity.
All developers are reminded to follow the ancil working practices
Branch
Related branches (e.g. ancillary-file-science):
[please link any related branches here]
ANTS rose stem logs
[please enter the workflow name here as it has been run e.g. ants-trunk/run5]
ancillary-file-science rose stem logs
[please enter the workflow name here as it has been run e.g. ancillary-file-science/run1]
Testing
For core ANTS only tests, the bare minimum that will be accepted is the
group=unittestsbut many, if not most, changes will need to test other groups to ensure they meet reviewer expectations. In general, it should be possible and is advised to run thegroup=allgroup prior to review submission as this will catch any consequential issues. Additionally you must run theancillary-file-sciencetests, pointing at your branch, withgroup=allto capture any behaviour changes affecting Science codes.If your change will alter existing science results, you will need to seek appropriate Scientific validation and confirm that the model has been initialised with your new development. Inspecting a change in xconv/pyplot/visualiser of choice is not sufficient to demonstrate the model can be initialised from your file.
Impact of change
cylc vip ./rose-stem -z group=alltestscylc vip ./rose-stem -z group=alltestsApprovals for this change
New functionality further testing
These tasks must succeed for your ticket to pass review.
Other
Rose stem logs
Please copy in the contents of your trac_status.log file(s) below (found in the cylc-run directory for your rose stem run) to your rose-stem testing here. Note: if your changes lead to a change in answers, you must run
cylc vip ./rose-stem -z group=allto help ensure all affected configurations has been flagged up.