Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move stitching code to prediction module #21

Merged
merged 4 commits into from
Feb 12, 2025

Conversation

davidwilby
Copy link
Owner

@davidwilby davidwilby commented Jan 27, 2025

@MartinSJRogers - Tom and I are keen to move some of the patch stitching to the pred module from model since it operates on predictions.

Tests are passing but could you take a look a this please? In particular some of the docstrings and the descriptions of X_t to advise me of any changes as I'm not sure these make sense.

While we're at it, the docstring of _get_coordinate_extent I'm not really sure I follow:

Get coordinate extent of dataset. This method is applied to either X_t or patchwise predictions.

Could you suggest something more descriptive please?

Finally, should we also consider moving get_patch_overlap to the same module? And possibly moving its call inside stitch_clipped_predictions?

Copy link
Collaborator

@MartinSJRogers MartinSJRogers left a comment

Choose a reason for hiding this comment

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

Approved- a few minor amendments to mark up text found in my PR: https://github.com/davidwilby/deepsensor/tree/mr_move_stitching_to_pred

@davidwilby davidwilby merged commit c114926 into patchwise_train Feb 12, 2025
6 of 8 checks passed
@davidwilby davidwilby deleted the dw/move_stitching_to_predict branch February 12, 2025 10:29
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