grass-addons
grass-addons copied to clipboard
Add i.saocom and i.sar toolsets
The i.saocom.toolset is intended to read SAOCOM-1A L1A images. It currently comprises two basic modules:
- i.saocom.import to import SAOCOM SLC data as real and imaginary bands for the specified polarimetric channels
- i.saocom.geocode to project these images or derived products from radar to projected coordinates
The use of the modules can be tried with this sample SAOCOM dataset. It can be processed either as a zip file or as an uncompressed folder.
The i.sar toolset in inteded for general use on any SAR image. It currently comprises three modules. One already existed as a stand-alone tool (i.sar.speckle), and the other two are new.
- i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.
- i.sar.amplitude: Tool intended to calculate an amplitude image from two SAR real and imaginary bands
- i.sar.pauliRGB: Tool intended to calculate the RGB bands used as input for the Pauli Composition
Thank you for these tools! While it is not a hard requirement at this point, it would be really good if you provide also tests for these tools. The tests are extremely useful for long-term maintenance and will soon become a crucial part of the day-to-day updates of the grass-addons repository. The tests can be written with 1) unittest-based tests which provide similar workflow experience as when writing a tool or an example or 2) pytest tests which are used experimentally now and may become more prominent later if successful.
Thanks for your contribution!
One thing:
i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.
Should the original src/imagery/i.sar.speckle/ be removed then? Otherwise we get (partially) duplicate code. What do you think @madi ?
Gracias @sase1988 !!
I can't help it to mention that this contribution is Santiago's final work for the GRASS GIS course where students were offered different options of final work to pass the course. Proud teacher and friend here! :star_struck:
Thanks for your contribution!
One thing:
i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.
Should the original
src/imagery/i.sar.speckle/be removed then? Otherwise we get (partially) duplicate code. What do you think @madi ?
Hello Markus! My proposal is to do so, in order to have all the SAR tools inside the same toolset. Of course, after the addon is approved :smile:
Having "all the SAR tools inside the same toolset" sounds reasonable.
While potentially complicating this PR, the clearest way might be to resolve this within one PR given that the name is the same. The issue with this in general is that moving a tool under a group breaks g.extension the.tool.name.
We don't give any promises on which addons install (although maybe we should), so formally it is okay, but still, we are trying hard to keep things working in general. With this in mind, perhaps leaving the original in place, but marking it or even replacing it with the new one (100% duplication) is on the table from my perspective.
Cool stuff. I am very much looking forward to testing the new addons.
Thanks for your contribution!
One thing:
i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.
Should the original
src/imagery/i.sar.speckle/be removed then? Otherwise we get (partially) duplicate code. What do you think @madi ?
Ciao @neteler and all. I'm glad to see more cool stuff related to SAR! It makes complete sense to me to have everything in one place, feel free to remove the addon that I created if it is a replication.
Cheers
Thank you for these tools! While it is not a hard requirement at this point, it would be really good if you provide also tests for these tools. The tests are extremely useful for long-term maintenance and will soon become a crucial part of the day-to-day updates of the grass-addons repository. The tests can be written with 1) unittest-based tests which provide similar workflow experience as when writing a tool or an example or 2) pytest tests which are used experimentally now and may become more prominent later if successful.
Dear Vaclav,
Thanks a lot for your feedback. I am now working on building a test script for i.saocom.import based on the unittest structure.
- I've seen that there are many assert() methods that could be very useful. The script file I attach (I changed the extension to .txt) runs the import of a sample SAOCOM dataset and compares it to a reference raster map (
assertRastersEqual). It also checks if the imported raster exists (assertRasterExists). Does it make sense usingassertRastersEqualin this context? - The script can be run under certain conditions: That GRASS has been initiated in a mapset where the reference raster exists. Does this make sense? I've seen the same logic in other codes like test_r_centroids. In case it does, how or where should I indicate this "base conditions", how should sample data be available if the file are large in terms of disk space?
I would appreciate any guidance on this, I am quite new to this stuff of creating tests and I'm very interested in learning about them ;)
Cheers
@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.
The test file look good in terms of code.
...runs the import of a sample SAOCOM dataset and compares it to a reference raster map (
assertRastersEqual). It also checks if the imported raster exists (assertRasterExists). Does it make sense usingassertRastersEqualin this context?
Yes, that's exactly how these are supposed to be used.
The script can be run under certain conditions: That GRASS has been initiated in a mapset where the reference raster exists. Does this make sense?
Correct. That's the idea for grass.gunittest. Specifically, the tests are running with an experimental version of NC SPM sample dataset. It is like writing a GRASS tool (that also assumes a GRASS session connected to a location/project). Additionally, the connected location/project is always NC SPM. Sort of the same like we do for documentation.
There can be exceptions if that's automated.
In case it does, how or where should I indicate this "base conditions", ...
Whatever you need beyond NC SPM, you need to get yourself from the data directory. Ideally, you would use NC SPM and then get additional data (you can ignore the data in NC SPM and just operate in the current mapset).
It seems you are using a special GRASS location you created. I don't have a good example how to do that with grass.gunittest, but a standard switch with g.mapset should work. Perhaps you don't need that at all and just use NC SPM.
...how should sample data be available if the file are large in terms of disk space?
That's the big issue here. How big is the data? Ideally, you would have a tiny dataset, like a 2x2 raster and you work with that.
If there is no way around using something like 10s of MB files, then we may end up with simple automated tests which do just trivial tasks (like fail because file already exists) just to check that at the tool at least starts. In addition to that, we would have separate set of semi-automated tests which you would need to run manually to confirm the tool is working. This may be needed for the download portion anyway if it can't access a local file because hitting an API from CI may not be doable.
@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.
Dear Anna, thank you very much for you feedback.
Regarding the detail information, where or how (which format) should I give a more in-depth explanation? As suggested by @veroandreo I am drawing some graphical workflows to add to the .html files, but the detail about the external dependencies and how they are used in each step, and why I prefer not using more low level bindings such as GDAL, might be too much for the tool final user's document.
@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.
Dear Anna, thank you very much for you feedback.
Regarding the detail information, where or how (which format) should I give a more in-depth explanation? As suggested by @veroandreo I am drawing some graphical workflows to add to the .html files, but the detail about the external dependencies and how they are used in each step, and why I prefer not using more low level bindings such as GDAL, might be too much for the tool final user's document.
Right, I think discussing it right here in the PR is appropriate. I still need to look at it in more detail, but it looks like you import GDAL package, but don't use it and instead call gdalwarp tool. Any reason why you use affine instead of rasterio.affine? In any case, the dependencies should be communicated in the manual and there should be lazy loading otherwise the addons won't be built.
@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.
Dear Anna, thank you very much for you feedback. Regarding the detail information, where or how (which format) should I give a more in-depth explanation? As suggested by @veroandreo I am drawing some graphical workflows to add to the .html files, but the detail about the external dependencies and how they are used in each step, and why I prefer not using more low level bindings such as GDAL, might be too much for the tool final user's document.
Right, I think discussing it right here in the PR is appropriate. I still need to look at it in more detail, but it looks like you import GDAL package, but don't use it and instead call gdalwarp tool. Any reason why you use affine instead of rasterio.affine? In any case, the dependencies should be communicated in the manual and there should be lazy loading otherwise the addons won't be built.
Dear Anna,
I am working on replacing the use of rasterio with GDAL. It makes the code a little bit longer, but in the end it will use GDAL only. The main complication is to find a GDAL simple method to do the same as rasterio.transform.from_gcps(). I have also eliminated many libraries that indeed were unused.
Dear Anna, I am working on replacing the use of rasterio with GDAL. It makes the code a little bit longer, but in the end it will use GDAL only. The main complication is to find a GDAL simple method to do the same as
rasterio.transform.from_gcps(). I have also eliminated many libraries that indeed were unused.
I don't insist on replacing rasterio if that's a problem... I was trying to understand which dependencies are necessary and which ones were used e.g. just because of a better syntax. Generally fewer extra dependencies makes it simpler for the user.
Looking at the diagrams, I was trying to understand the workflow and this is still unclear, I may be missing something: Why are these 2 modules and not one, just the geocode? What is the purpose of importing it into a temporary XY location and then exporting tif from there, doing the geocoding via external tools and then importing it again, it seems the temporary XY location step is not doing anything?
Also, as Vero noted earlier, the geocode tool should import it to the current location, not create a new one and import it there. Is there any reason it needs to be epsg 4326, it seems at that point any CRS should be acceptable, no?