search

opera-adt / COMPASS

COregistered Multi-temPorAl Sar Slc
Apache License 2.0
38 stars 18 forks source link

replace real/imag stats with power/phase stats #150

Closed LiangJYu closed 1 year ago

LiangJYu commented 1 year ago

This PR aims to make QA stats for CSLC consistent with that of *SLC in NISAR QA. To expedite the merging of this PR, the stats (min, max, mean, stddev) are computed with numpy. The isce3.math.stats class with its multithreading capability can not be as easily adapted to apply power and phase to a complex raster.

TODO: ~testing~

vbrancat commented 1 year ago

@LiangJYu make sure to add reviewers

vbrancat commented 1 year ago

@LiangJYu is testing still pending?

LiangJYu commented 1 year ago

@LiangJYu is testing still pending?

Good to go. I think #148 should take precedence over this PR to prevent conflicts there.

LiangJYu commented 1 year ago

Need to update specifile.txt with pytest-order

LiangJYu commented 1 year ago

These unit tests are elementary. The code should be seen as a placeholder instead of as the final test. I added them with the idea that it's better to have some test than no test. How do you reviewers feel about this approach?

seongsujeong commented 1 year ago

These unit tests are elementary. The code should be seen as a placeholder instead of as the final test. I added them with the idea that it's better to have some test than no test. How do you reviewers feel about this approach?

Fine with me. I think we can put the tests with some relexed thresholds, and revisit this to fine-tune the criteria in the tests.

seongsujeong commented 1 year ago

These unit tests are elementary. The code should be seen as a placeholder instead of as the final test. I added them with the idea that it's better to have some test than no test. How do you reviewers feel about this approach?

Fine with me. I think we can put the tests with some relexed thresholds, and revisit this to fine-tune the criteria in the tests.

But just letting you know that the test on the mean of phase failed. Below is the code snippet I've added couple of lines like the snipped below to print out the stats:

print(f'Testing: {stat_name} of {pwr_phase}', end=' ')
stat_val =h5_obj[f'{h5_stats_path}/{stat_name}'][()]
print(stat_val)

And here is what the value looks like:

Testing: mean of power 2552.5999
Testing: min of power 2.6862714e-05
Testing: max of power 56852844.0
Testing: mean of phase -0.00043806355

So apparently the current criteria for the phase mean needs to be adjusted. Please see my comment above.