Skip to content

Conversation

@scottstanie
Copy link
Member

@scottstanie scottstanie commented Jan 30, 2026

  1. Fixed edge handling - Changed from range(0, shape - psize, step) to proper padding that covers all pixels
  2. Reflection padding on all sides - Instead of zero-padding (which causes artifacts), uses np.pad(..., mode="reflect") to maintain signal continuity at edges
  3. Weight normalization - Added weight_sum accumulator to properly normalize the output, especially important at edges where fewer windows overlap
  4. Fixed empty_mask - Changed from np.angle(data) == 0 (which incorrectly treats valid phase=0 as empty) to data == 0 (only treats zero-magnitude complex values as empty)

Before:
image

After:
image

Change Summary

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review

1. Fixed edge handling - Changed from range(0, shape - psize, step) to proper padding that covers all pixels
2. Reflection padding on all sides - Instead of zero-padding (which causes artifacts), uses np.pad(..., mode="reflect") to maintain signal continuity at edges
3. Weight normalization - Added weight_sum accumulator to properly normalize the output, especially important at edges where fewer windows overlap
4. Fixed empty_mask - Changed from np.angle(data) == 0 (which incorrectly treats valid phase=0 as empty) to data == 0 (only treats zero-magnitude complex values as empty)
Copy link
Collaborator

@mirzaees mirzaees left a comment

Choose a reason for hiding this comment

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

Thanks @scottstanie

@mirzaees mirzaees merged commit 47052b4 into isce-framework:main Feb 11, 2026
5 checks passed
@scottstanie scottstanie deleted the fix-goldstein-edge branch February 11, 2026 17:56
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