-
Notifications
You must be signed in to change notification settings - Fork 85
Fix the Ampcor issues from Lijun's branch and increase the limit of the search radius #282
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
Merged
hfattahi
merged 38 commits into
isce-framework:develop
from
xhuang-jpl:fix_ampcor_issue1
Jun 24, 2026
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
da95af1
sm r3.4
3a8e2ec
merge R4.0.2beta-1
b6cfbbb
change the SM commit id for R4.0.2
f764f2f
merge upstream develop branch
a920ae6
Merge remote-tracking branch 'upstream/develop' into develop
1d28fae
Merge remote-tracking branch 'upstream/develop' into develop
84f3874
Merge remote-tracking branch 'upstream/develop' into develop
2bb897a
Merge remote-tracking branch 'upstream/develop' into develop
da76f88
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
6313217
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
ce2b301
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
8394511
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
72645bd
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
6f93fd3
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
eebd988
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
9b108ce
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
b1c78f7
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
e46f53f
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
54d6ab4
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
73b6b9b
Merge branch 'develop' of https://github-fn.jpl.nasa.gov/isce-3/isce …
2eb1387
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
42f2514
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
39c9883
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
f6cfd55
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
6176caa
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
1f111b3
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
0cf5e3b
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
41199fb
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
a0bbee4
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
948b25b
remove the mask description for the wrapped ifgram
a3bddfe
remove the tmp folder
aa375c3
fix minors
24ffd1c
comment out the checkPixelInImageRange
3e0839d
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
0c9d6a6
pycuampcor: fix the pixels out of range issue
f7c69c1
pycuampcor: fix an error in determining the corrwindow size
b236902
Merge branch 'develop' of https://github.com/isce-framework/isce3 int…
cfbdf5a
update the maximum search radius to be 256
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,7 +68,7 @@ void cuArraysCopyToBatch(cuArrays<float2> *image1, cuArrays<float2> *image2, | |
|
|
||
| // kernel for cuArraysCopyToBatchWithOffset | ||
| template<typename T_in, typename T_out> | ||
| __global__ void cuArraysCopyToBatchWithOffset_kernel(const T_in *imageIn, const int inNY, | ||
| __global__ void cuArraysCopyToBatchWithOffset_kernel(const T_in *imageIn, const int inNX, const int inNY, | ||
| T_out *imageOut, const int outNX, const int outNY, const int nImages, | ||
| const int *offsetX, const int *offsetY) | ||
| { | ||
|
|
@@ -77,8 +77,12 @@ __global__ void cuArraysCopyToBatchWithOffset_kernel(const T_in *imageIn, const | |
| int outy = threadIdx.y + blockDim.y*blockIdx.y; | ||
| if(idxImage>=nImages || outx >= outNX || outy >= outNY) return; | ||
| int idxOut = idxImage*outNX*outNY + outx*outNY + outy; | ||
| int idxIn = (offsetX[idxImage]+outx)*inNY + offsetY[idxImage] + outy; | ||
| imageOut[idxOut] = T_out{imageIn[idxIn]}; | ||
| int inx = offsetX[idxImage] + outx; | ||
| int iny = offsetY[idxImage] + outy; | ||
| if (inx >= 0 && inx < inNX && iny >= 0 && iny < inNY) | ||
| imageOut[idxOut] = T_out{imageIn[inx*inNY + iny]}; | ||
| else | ||
| imageOut[idxOut] = T_out{0.0f}; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -98,14 +102,14 @@ void cuArraysCopyToBatchWithOffset(cuArrays<float2> *image1, const int lda1, cuA | |
| dim3 blockSize(nthreads, nthreads, 1); | ||
| dim3 gridSize(IDIVUP(image2->height,nthreads), IDIVUP(image2->width,nthreads), image2->count); | ||
| cuArraysCopyToBatchWithOffset_kernel<<<gridSize,blockSize, 0 , stream>>> ( | ||
| image1->devData, lda1, | ||
| image1->devData, image1->height, lda1, | ||
| image2->devData, image2->height, image2->width, image2->count, | ||
| offsetH, offsetW); | ||
| getLastCudaError("cuArraysCopyToBatchAbsWithOffset_kernel"); | ||
| } | ||
|
|
||
| // same as above, but from complex to real(take amplitudes) | ||
| __global__ void cuArraysCopyToBatchAbsWithOffset_kernel(const float2 *imageIn, const int inNY, | ||
| __global__ void cuArraysCopyToBatchAbsWithOffset_kernel(const float2 *imageIn, const int inNX, const int inNY, | ||
| float2 *imageOut, const int outNX, const int outNY, const int nImages, | ||
| const int *offsetX, const int *offsetY) | ||
| { | ||
|
|
@@ -114,8 +118,12 @@ __global__ void cuArraysCopyToBatchAbsWithOffset_kernel(const float2 *imageIn, c | |
| int outy = threadIdx.y + blockDim.y*blockIdx.y; | ||
| if(idxImage>=nImages || outx >= outNX || outy >= outNY) return; | ||
| int idxOut = idxImage*outNX*outNY + outx*outNY + outy; | ||
| int idxIn = (offsetX[idxImage]+outx)*inNY + offsetY[idxImage] + outy; | ||
| imageOut[idxOut] = make_float2(complexAbs(imageIn[idxIn]), 0.0); | ||
| int inx = offsetX[idxImage] + outx; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is an example that range is checked during copying and zeros are filled when out of range happens. |
||
| int iny = offsetY[idxImage] + outy; | ||
| if (inx >= 0 && inx < inNX && iny >= 0 && iny < inNY) | ||
| imageOut[idxOut] = make_float2(complexAbs(imageIn[inx*inNY + iny]), 0.0f); | ||
| else | ||
| imageOut[idxOut] = make_float2(0.0f, 0.0f); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -135,7 +143,7 @@ void cuArraysCopyToBatchAbsWithOffset(cuArrays<float2> *image1, const int lda1, | |
| dim3 blockSize(nthreads, nthreads, 1); | ||
| dim3 gridSize(IDIVUP(image2->height,nthreads), IDIVUP(image2->width,nthreads), image2->count); | ||
| cuArraysCopyToBatchAbsWithOffset_kernel<<<gridSize,blockSize, 0 , stream>>> ( | ||
| image1->devData, lda1, | ||
| image1->devData, image1->height, lda1, | ||
| image2->devData, image2->height, image2->width, image2->count, | ||
| offsetH, offsetW); | ||
| getLastCudaError("cuArraysCopyToBatchAbsWithOffset_kernel"); | ||
|
|
@@ -158,7 +166,7 @@ void cuArraysCopyToBatchWithOffsetR2C(cuArrays<float> *image1, const int lda1, c | |
| dim3 blockSize(nthreads, nthreads, 1); | ||
| dim3 gridSize(IDIVUP(image2->height,nthreads), IDIVUP(image2->width,nthreads), image2->count); | ||
| cuArraysCopyToBatchWithOffset_kernel<<<gridSize,blockSize, 0 , stream>>> ( | ||
| image1->devData, lda1, | ||
| image1->devData, image1->height, lda1, | ||
| image2->devData, image2->height, image2->width, image2->count, | ||
| offsetH, offsetW); | ||
| getLastCudaError("cuArraysCopyToBatchWithOffsetR2C_kernel"); | ||
|
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the previous behavior was to raise an exception when this check failed. The PR changes that behavior to emit a warning message instead. Could you explain the rationale behind this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point @seongsujeong , it is because they are not the issue anymore, and just warn the user that the window might out of range. @lijun99 has done the fix via filling 0. Please check his reply:
The isce3 version uses a strict range check, which could fail when we choose a specific starting pixel and some pixels from search image may fall out of range (at the edges). I have patched the code from the standalone version, which fills (0,0) when using pixels out of range. Now checkPixelInRange is optional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Image loading/copying, zeros are now filled for out of range pixels (happening at edges if margins are not set properly or if we want to use a fixed start pixel location and vary chip sizes or search ranges). Of course, if it happens, the results along the edge are not reliable and therefore warnings may be issued. But it should not prevent the program from running.
Before the revision, accessing out of range pixels leads to memory access errors (seg fault, at least for CUDA), and therefore checking the range and raising an exception was the only plausible solution.
Note also that the above bug fixes have been included in the PyCuAmpcor V2 PR. Somehow, I forgot to patch V1. So, current version of ISCE2 has similar issues.