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

Process restructuring #63

Merged
merged 25 commits into from Sep 28, 2022
Merged

Process restructuring #63

merged 25 commits into from Sep 28, 2022

Conversation

johntruckenbrodt
Copy link
Member

@johntruckenbrodt johntruckenbrodt commented Sep 14, 2022

This is a major restructuring to solve several shortcomings:

  • the processor now accepts geometries spanning multipe UTM zones (Enable processing across multiple UTM zones #32)
  • the SNAP processing has been outsourced to a new module snap
  • SNAP processing is now much more efficient by
    • combining all procesing steps of pyroSAR.snap.geocode and pyroSAR.snap.noise_power (Optimize noise power processing #39)
    • reusing intermediate processing output for multi-UTM processing
    • geocoding only to areas that are needed for the current UTM zone
  • no longer create DEM MGRS tiles but rather only create scene-specific mosaics in EPSG:4326 (the MGRS tiling was done to save time because EGM2008 conversion used to be very slow. This is no longer an issue nad the storage space can thus be saved)

@maawoo
Copy link
Member

maawoo commented Sep 16, 2022

Great, thanks John! Is it still work in progress? I could do some tests later today.

@johntruckenbrodt
Copy link
Member Author

Hi Marco. This is almost ready. However, so far I have only fully tested it with IW GRD images because for SLCs I got a weird SNAP error during terrain flattening, something about slant range - ground range lookup tables. Haven't figured out what the source is yet.
You are very welcome to give it a try. I was going to ask you to revise the logging functionality. The pyroSAR log messages are now printed twice. I don't quite understand your logging module yet 😅.

@maawoo
Copy link
Member

maawoo commented Sep 16, 2022

The duplicate log messages are now fixed and I added a few other improvements.

Error: SARGeoCoding: srgrConvParams not set is the one you also get when processing SLC, right?
TF works with tf.parameters['outputSimulatedImage'] = False, but other than that I didn't find anything else 😕

@johntruckenbrodt
Copy link
Member Author

Sorry @maawoo, I forgot to mention that this PR depends on changes in pyroSAR in johntruckenbrodt/pyroSAR#225 and https://github.com/johntruckenbrodt/pyroSAR/tree/feature/dem_enhance. It seems you have figured that out yourself anyways though 💪.

@maawoo
Copy link
Member

maawoo commented Sep 19, 2022

Yes, I figured that out 🙂
The problem also occurs in the SNAP GUI and using an auto-downloaded DEM. So two factors that can be ruled out.

Screenshot 2022-09-19 at 10 49 14

@maawoo
Copy link
Member

maawoo commented Sep 19, 2022

I just had a look into the TF source code looking for srgrConvParams and I think line 812 needs to be moved after the if (isGRD) clause in line 820. But maybe I'm wrong 😅
Anyway, I think this issue is out of our hands and needs to be reported.

@johntruckenbrodt
Copy link
Member Author

Thanks @maawoo for having a look. I have created an issue:
https://forum.step.esa.int/t/terrain-flattening-output-simulated-image-fails-for-s1-slcs/37359

@johntruckenbrodt johntruckenbrodt merged commit 3991637 into main Sep 28, 2022
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.

Optimize noise power processing Enable processing across multiple UTM zones
2 participants