[GRASS-dev] ​GSoC 2018 report week 03 - GRASS GIS module for Sentinel-2 cloud and shadow detection

Hi all!
I’m Roberta Fagandini and I’m working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.

This is my report for the third week of coding.

1) What did I complete this week?

  • Implemented some changes from dev feedback (e.g. r.univar instead of r.stats.zonal)

  • Tested the modified python script and fixed bugs

  • Prepared the python script in order to start implementing the GUI

  • Started implementing the GUI

  • Made some changes to the code depending on the GUI requirements (add controls and check on input, output and temporary file, etc.)

  • Cleaned up the whole code

  • Tested the GUI and fixed bugs

  • Frequently added the basic version of the GUI to my GitHub repository [0]

  • Shared progress with the community

2) What am I going to achieve for next week?

  • Implement any change from discussions and feedback

  • Test and fix bugs

  • Finish the implementation of the GUI

  • Start writing the manual page

3) Is there any blocking issue?

No at the moment.

Here the links to my wiki page [1] and GitHub repository [0]

Kind regards,
Roberta

[0] https://github.com/RobiFag/GRASS_clouds_and_shadows

[1] https://trac.osgeo.org/grass/wiki/GSoC/2018/CloudsAndShadowsDetection

Nice progress!

Quick question: Would it make sense to split the module into one cloud-detection and one cloud shadow detection module? Or are both processes interdependent?

Cheers,

Stefan

···

Hi all!

I’m Roberta Fagandini and I’m working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.

This is my report for the third week of coding.

1) What did I complete this week?

· Implemented some changes from dev feedback (e.g. r.univar instead of r.stats.zonal)

· Tested the modified python script and fixed bugs

· Prepared the python script in order to start implementing the GUI

· Started implementing the GUI

· Made some changes to the code depending on the GUI requirements (add controls and check on input, output and temporary file, etc.)

· Cleaned up the whole code

· Tested the GUI and fixed bugs

· Frequently added the basic version of the GUI to my GitHub repository [0]

· Shared progress with the community

2) What am I going to achieve for next week?

· Implement any change from discussions and feedback

· Test and fix bugs

· Finish the implementation of the GUI

· Start writing the manual page

3) Is there any blocking issue?

No at the moment.

Here the links to my wiki page [1] and GitHub repository [0]

Kind regards,

Roberta

[0] https://github.com/RobiFag/GRASS_clouds_and_shadows

[1] https://trac.osgeo.org/grass/wiki/GSoC/2018/CloudsAndShadowsDetection

Hi Stefan,
thank you for the question!!
The two procedures are not completely indipendent from each other, indeed the shadows mask relies on the computation of the clouds mask for the “cleaning” phase. Maybe it could make sense to manage both procedures using a flag. In this way users can decide via GUI whether to compute only the clouds mask or both.

What do you think? Would it be helpful?

Ciao
Roberta

···

2018-06-03 15:04 GMT+02:00 Stefan Blumentrath <Stefan.Blumentrath@nina.no>:

Nice progress!

Quick question: Would it make sense to split the module into one cloud-detection and one cloud shadow detection module? Or are both processes interdependent?

Cheers,

Stefan

From: grass-dev <grass-dev-bounces@lists.osgeo.org> On Behalf Of Roberta Fagandini
Sent: søndag 3. juni 2018 14:39
To: soc@lists.osgeo.org; GRASS developers list <grass-dev@lists.osgeo.org>
Subject: [GRASS-dev] ​GSoC 2018 report week 03 - GRASS GIS module for Sentinel-2 cloud and shadow detection

Hi all!

I’m Roberta Fagandini and I’m working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.

This is my report for the third week of coding.

1) What did I complete this week?

· Implemented some changes from dev feedback (e.g. r.univar instead of r.stats.zonal)

· Tested the modified python script and fixed bugs

· Prepared the python script in order to start implementing the GUI

· Started implementing the GUI

· Made some changes to the code depending on the GUI requirements (add controls and check on input, output and temporary file, etc.)

· Cleaned up the whole code

· Tested the GUI and fixed bugs

· Frequently added the basic version of the GUI to my GitHub repository [0]

· Shared progress with the community

2) What am I going to achieve for next week?

· Implement any change from discussions and feedback

· Test and fix bugs

· Finish the implementation of the GUI

· Start writing the manual page

3) Is there any blocking issue?

No at the moment.

Here the links to my wiki page [1] and GitHub repository [0]

Kind regards,

Roberta

[0] https://github.com/RobiFag/GRASS_clouds_and_shadows

[1] https://trac.osgeo.org/grass/wiki/GSoC/2018/CloudsAndShadowsDetection

Hi Roberta !

On 03/06/18 14:38, Roberta Fagandini wrote:

Hi all!
I'm Roberta Fagandini and I'm working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.
This is my report for the third week of coding.

*1) What did I complete this week?*

  * Implemented some changes from dev feedback (e.g. r.univar instead of
    r.stats.zonal)
  * Tested the modified python script and fixed bugs
  * Prepared the python script in order to start implementing the GUI
  * Started implementing the GUI
  * Made some changes to the code depending on the GUI requirements (add
    controls and check on input, output and temporary file, etc.)
  * Cleaned up the whole code
  * Tested the GUI and fixed bugs
  * Frequently added the basic version of the GUI to my
    GitHub repository [0]
  * Shared progress with the community

I'm sorry, I haven't had the time to test your module, yet. A few recommendation, though, based on a quick read of the code. I assume that the relevant file is i.sentinel.mask.

- I would recommend to immediately create a real complete module that can be installed with g.extension. This means creating a directory with the module as .py file, a .html manual page file, but which could be empty at the beginning, and a Makefile (see existing scripts in the core code or in addons for examples). Being able to install everything directly from github with a simple g.extension call makes testing much easier.

- I see that all the input bands are marked as required. Are they all necessary to the calculations ? If yes, aren't all these bands always from the same acquisition ? If yes, then maybe we can avoid parameter inflation by either just asking for a prefix and then getting all bands from that prefix (following standard sentinel naming), or provide the MTD file and read the band names from there, or you could ask for a group name and it would be up to the user to create the group before launching the module.

- A small idea in terms of coding style: I personally think that code should be as easily understandable as possible. For example: in the formulas you use the f_bands list. Maybe you could make this into a dictionary and so instead of reading f_bands[0], we would read f_bands['blue'] making the formulas easier to follow.

- Please follow PEP8 style guide (see [1]). Amongst others, this means limiting lines to 79 characters. So:

  - The general style used for gscript.*_command calls in Python scripts is

  gscript.run_command('v.dissolve',
          input=tmp["centroid"],
          column='value',
          output=tmp["dissolve"],
          overwrite=True,
          quiet=True)

  i.e. each parameter on its own line. This make it easier to read
  and lines shorter.

  - The same for r.mapcalc calls: if necessary you can construct
  the individual rules over more than one line.

I'll try to find some time this week to test the module and maybe come back with some more feedback on the coding.

Great job so far, though !

Moritz

[1] https://trac.osgeo.org/grass/wiki/Submitting/Python#Style

Ciao Roberta,

great work!

Moritz got me faster, as I intended to chime in, with similar
suggestions.

So +1 for:

- a "real" module, as Moritz described it

- for using the metadata file, and parsing it -- my experience with
  Landsat and other high-resolution satellite products, showed it's best to
  trust the individual metadata that accompany an acquisition.
    Parameters may be very specific to one single acquisition.
  Also, a module that is designed to be "dynamic" is more
  future-proof and easy to maintain and update.
    For example, reading band names from the meta-data, as opposed to more
  "static" designs, i.e. expecting specific hardcoded options, such as
  band names, or their order.

- PEP8 rules and keeping module options in separate lines to increase legibility

In line with the suggestions on style, I would stress out to fully
spell out the names of functions, options, descriptions of flags,
iterables and iterators. It will make it easier for the next reader of
your code.

Finally, it's a good idea to write even mini-tests, while you are
progressing, for the small functions that eventually develop to keep the
full scrip compact, functional and more legible.

Writing a test, for your module, in the end, will be much easier.

I am sure you know all this. Yet, discussing it is a good thing in
my view.

Keep up the good work, Nikos

* Moritz Lennert <mlennert@club.worldonline.be> [2018-06-04 11:03:41 +0200]:

Hi Roberta !

On 03/06/18 14:38, Roberta Fagandini wrote:

Hi all!
I'm Roberta Fagandini and I'm working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.
This is my report for the third week of coding.

*1) What did I complete this week?*

* Implemented some changes from dev feedback (e.g. r.univar instead of
   r.stats.zonal)
* Tested the modified python script and fixed bugs
* Prepared the python script in order to start implementing the GUI
* Started implementing the GUI
* Made some changes to the code depending on the GUI requirements (add
   controls and check on input, output and temporary file, etc.)
* Cleaned up the whole code
* Tested the GUI and fixed bugs
* Frequently added the basic version of the GUI to my
   GitHub repository [0]
* Shared progress with the community

I'm sorry, I haven't had the time to test your module, yet. A few recommendation, though, based on a quick read of the code. I assume that the relevant file is i.sentinel.mask.

- I would recommend to immediately create a real complete module that can be installed with g.extension. This means creating a directory with the module as .py file, a .html manual page file, but which could be empty at the beginning, and a Makefile (see existing scripts in the core code or in addons for examples). Being able to install everything directly from github with a simple g.extension call makes testing much easier.

- I see that all the input bands are marked as required. Are they all necessary to the calculations ? If yes, aren't all these bands always from the same acquisition ? If yes, then maybe we can avoid parameter inflation by either just asking for a prefix and then getting all bands from that prefix (following standard sentinel naming), or provide the MTD file and read the band names from there, or you could ask for a group name and it would be up to the user to create the group before launching the module.

- A small idea in terms of coding style: I personally think that code should be as easily understandable as possible. For example: in the formulas you use the f_bands list. Maybe you could make this into a dictionary and so instead of reading f_bands[0], we would read f_bands['blue'] making the formulas easier to follow.

- Please follow PEP8 style guide (see [1]). Amongst others, this means limiting lines to 79 characters. So:

- The general style used for gscript.*_command calls in Python scripts is

gscript.run_command('v.dissolve',
        input=tmp["centroid"],
        column='value',
        output=tmp["dissolve"],
        overwrite=True,
        quiet=True)

i.e. each parameter on its own line. This make it easier to read
and lines shorter.

- The same for r.mapcalc calls: if necessary you can construct
the individual rules over more than one line.

I'll try to find some time this week to test the module and maybe come back with some more feedback on the coding.

Great job so far, though !

Moritz

[1] https://trac.osgeo.org/grass/wiki/Submitting/Python#Style

_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

--
Nikos Alexandris | Remote Sensing & Geomatics
GPG Key Fingerprint 6F9D4506F3CA28380974D31A9053534B693C4FB3

Hi all!

Thank you, Moritz and Nikos, for your suggestions!

···

2018-06-04 11:54 GMT+02:00 Nikos Alexandris <nik@nikosalexandris.net>:

Ciao Roberta,

great work!

Moritz got me faster, as I intended to chime in, with similar
suggestions.

So +1 for:

  • a “real” module, as Moritz described it

Done! I added the folder to my github repository but I have some trouble compiling the module with g.extension:

g.extension extension=i.sentinel.mask operation=add url=https://github.com/RobiFag/GRASS_clouds_and_shadows

error message:

g.extension extension=i.sentinel.mask operation=add url=https://github.com/RobiFag/GRASS_clouds_and_shadows
Fetching <i.sentinel.mask> from <https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip> (be patient)…
Compiling…
make[1]: *** No rule to make target /tmp/grass7-roberta-701 4/tmpe7QEso/i.sentinel.mask/scripts/i.sentinel.mask', needed by script’. Stop.
/bin/sh: 1: cannot create /usr/lib/grass74/error.log:
Permission denied
make: *** [i.sentinel.mask] Error 2
ERROR: Compilation failed, sorry. Please check above error messages.

any suggestion?

  • for using the metadata file, and parsing it – my experience with
    Landsat and other high-resolution satellite products, showed it’s best to
    trust the individual metadata that accompany an acquisition.
    Parameters may be very specific to one single acquisition.
    Also, a module that is designed to be “dynamic” is more
    future-proof and easy to maintain and update.
    For example, reading band names from the meta-data, as opposed to more
    “static” designs, i.e. expecting specific hardcoded options, such as
    band names, or their order.

I have already thought about parsing the metadatafile to retrieve the input bands but at the moment the module requires atmospherically corrected bands and if users perform the atmospheric correction on their own I don’t have any control on naming. Moreover, the module needs to know which raster map corresponds to the blue band, to the green band and so on in order to apply the rules for clouds and shadows detection. Therefore I need users specify each band but maybe I can manage this issue requiring a specific suffix in the bands’ names (e.g. blabla_blue, blabla_green, etc.). In this way, users have to provide only the prefix (blabla) while the module search for all raster maps with the specified prefix and at the same time it is able to recognize each band.

Anyway, the aim of the next coding period is to create a python script that wraps in a single GRASS module the download and import phase (i.sentinel.download and i.sentinel.import), the atmospheric correction using i.atcorr and the cloud and shadow detection procedure. In particular, for what concerns the atmospheric correction, I’d like to implement an iterative procedure that executes i.atcorr for all bands of the input image changing accordingly the requested input parameters. This part should simplify the management of input maps.

  • PEP8 rules and keeping module options in separate lines to increase legibility

In line with the suggestions on style, I would stress out to fully
spell out the names of functions, options, descriptions of flags,
iterables and iterators. It will make it easier for the next reader of
your code.

Yes, you are both right!! I have to clean up the code from the style point of view and it will be one of the tasks of this week and especially of the last days before the GSoC Evaluation.

Finally, it’s a good idea to write even mini-tests, while you are
progressing, for the small functions that eventually develop to keep the
full scrip compact, functional and more legible.

Writing a test, for your module, in the end, will be much easier.

Sorry Nikos but I don’t really understand what you mean by “mini-tests”…are you referring to the examples in the manual page?

I am sure you know all this. Yet, discussing it is a good thing in
my view.

Discussing it is very useful especially for me so please do not take anything for granted and keep sending me your feedback!

Keep up the good work, Nikos

Cheers!
Roberta

Hi Roberta !

On 03/06/18 14:38, Roberta Fagandini wrote:

Hi all!
I’m Roberta Fagandini and I’m working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.
This is my report for the third week of coding.

1) What did I complete this week?

  • Implemented some changes from dev feedback (e.g. r.univar instead of
    r.stats.zonal)
  • Tested the modified python script and fixed bugs
  • Prepared the python script in order to start implementing the GUI
  • Started implementing the GUI
  • Made some changes to the code depending on the GUI requirements (add
    controls and check on input, output and temporary file, etc.)
  • Cleaned up the whole code
  • Tested the GUI and fixed bugs
  • Frequently added the basic version of the GUI to my
    GitHub repository [0]
  • Shared progress with the community

I’m sorry, I haven’t had the time to test your module, yet. A few recommendation, though, based on a quick read of the code. I assume that the relevant file is i.sentinel.mask.

  • I would recommend to immediately create a real complete module that can be installed with g.extension. This means creating a directory with the module as .py file, a .html manual page file, but which could be empty at the beginning, and a Makefile (see existing scripts in the core code or in addons for examples). Being able to install everything directly from github with a simple g.extension call makes testing much easier.

  • I see that all the input bands are marked as required. Are they all necessary to the calculations ? If yes, aren’t all these bands always from the same acquisition ? If yes, then maybe we can avoid parameter inflation by either just asking for a prefix and then getting all bands from that prefix (following standard sentinel naming), or provide the MTD file and read the band names from there, or you could ask for a group name and it would be up to the user to create the group before launching the module.

  • A small idea in terms of coding style: I personally think that code should be as easily understandable as possible. For example: in the formulas you use the f_bands list. Maybe you could make this into a dictionary and so instead of reading f_bands[0], we would read f_bands[‘blue’] making the formulas easier to follow.

  • Please follow PEP8 style guide (see [1]). Amongst others, this means limiting lines to 79 characters. So:

  • The general style used for gscript.*_command calls in Python scripts is

gscript.run_command(‘v.dissolve’,
input=tmp[“centroid”],
column=‘value’,
output=tmp[“dissolve”],
overwrite=True,
quiet=True)

i.e. each parameter on its own line. This make it easier to read
and lines shorter.

  • The same for r.mapcalc calls: if necessary you can construct
    the individual rules over more than one line.

I’ll try to find some time this week to test the module and maybe come back with some more feedback on the coding.

Great job so far, though !

Moritz

[1] https://trac.osgeo.org/grass/wiki/Submitting/Python#Style


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev


Nikos Alexandris | Remote Sensing & Geomatics
GPG Key Fingerprint 6F9D4506F3CA28380974D31A9053534B693C4FB3

* Roberta Fagandini <robifagandini@gmail.com> [2018-06-04 16:14:16 +0200]:

Hi all!

Thank you, Moritz and Nikos, for your suggestions!

2018-06-04 11:54 GMT+02:00 Nikos Alexandris <nik@nikosalexandris.net>:

Ciao Roberta,

great work!

Moritz got me faster, as I intended to chime in, with similar
suggestions.

So +1 for:

- a "real" module, as Moritz described it

Done! I added the folder to my github repository but I have some trouble
compiling the module with g.extension:

g.extension extension=i.sentinel.mask operation=add url=
https://github.com/RobiFag/GRASS_clouds_and_shadows

error message:
g.extension extension=i.sentinel.mask operation=add url=
https://github.com/RobiFag/GRASS_clouds_and_shadows
Fetching <i.sentinel.mask> from <
https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip&gt; (be
patient)...
Compiling...
make[1]: *** No rule to make target `/tmp/grass7-roberta-701
4/tmpe7QEso/i.sentinel.mask/scripts/i.sentinel.mask', needed
by `script'. Stop.
/bin/sh: 1: cannot create /usr/lib/grass74/error.log:
Permission denied
make: *** [i.sentinel.mask] Error 2
ERROR: Compilation failed, sorry. Please check above error messages.

any suggestion?

Maybe irrelevant, but perhaps it is best to name the script as
`i.sentinel.mask.py`?

- for using the metadata file, and parsing it -- my experience with
Landsat and other high-resolution satellite products, showed it's best to
trust the individual metadata that accompany an acquisition.
  Parameters may be very specific to one single acquisition.
Also, a module that is designed to be "dynamic" is more
future-proof and easy to maintain and update.
  For example, reading band names from the meta-data, as opposed to more
"static" designs, i.e. expecting specific hardcoded options, such as
band names, or their order.

I have already thought about parsing the metadatafile to retrieve the input
bands but at the moment the module requires atmospherically corrected bands
and if users perform the atmospheric correction on their own I don't have
any control on naming. Moreover, the module needs to know which raster map
corresponds to the blue band, to the green band and so on in order to apply
the rules for clouds and shadows detection. Therefore I need users specify
each band but maybe I can manage this issue requiring a specific suffix in
the bands' names (e.g. blabla_blue, blabla_green, etc.). In this way, users
have to provide only the prefix (blabla) while the module search for all
raster maps with the specified prefix and at the same time it is able to
recognize each band.

Anyway, the aim of the next coding period is to create a python script that
wraps in a single GRASS module the download and import phase
(i.sentinel.download and i.sentinel.import), the atmospheric correction
using i.atcorr and the cloud and shadow detection procedure. In particular,
for what concerns the atmospheric correction, I'd like to implement an
iterative procedure that executes i.atcorr for all bands of the input image
changing accordingly the requested input parameters. This part should
simplify the management of input maps.

One thing here: massive processing workflows like to have simple
processing building blocks. One Input -> one Process -> one Output. In
"your" case:

- Input: name of/request one band and associated
  metadata, and optionally other ancillary data.

- Process: download, import, removal of atmospheric effects to estimate
  surface reflectance

- Output: surface reflectance ("corrected" for atmospheric effects) of
  the requested input band

This building block can run in one processing units/node. Making use of many
processing units/nodes, this block can run in parallel for different
input bands.

I suggest to build the module in a way that it runs for one band or as many
bands as the user requires for his workflow. I did this in
https://gitlab.com/NikosAlexandris/i.fusion.hpf/blob/master/i.fusion.hpf.py.
I don't suggest this script as an example. Yet I think the concept "for
one or many" is useful and wanted.

As a sidenote:

I think that we don't use the term "correction"
correctly. An image acquired by a satellite-borne sensor is not "wrong"
to need a correction. Rather, we try to retouch the digital form of the
acquisition, in order to estimate the energy that was actually reflected
by the surface, before the signal travels through the atmosphere.

I studied remote sensing with Thomas Lillesand's; Ralph W.
Kiefer's book "Remote Sensing and Image Interpretation". The book
"correctly" touches this very question.

Maybe it's only a perspective, and others have another say on this.

- PEP8 rules and keeping module options in separate lines to increase
legibility

In line with the suggestions on style, I would stress out to fully
spell out the names of functions, options, descriptions of flags,
iterables and iterators. It will make it easier for the next reader of
your code.

Yes, you are both right!! I have to clean up the code from the style point
of view and it will be one of the tasks of this week and especially of the
last days before the GSoC Evaluation.

Finally, it's a good idea to write even mini-tests, while you are
progressing, for the small functions that eventually develop to keep the
full scrip compact, functional and more legible.

Writing a test, for your module, in the end, will be much easier.

Sorry Nikos but I don't really understand what you mean by
"mini-tests"..are you referring to the examples in the manual page?

For a function f(x) that takes x as an input and gives you y as output,
test, for example, whether the output value 'y' is within the expected
range. Test your single functions, inside the script. Use assertions, for
example, etc. If you have 10 functions in your script, and have written
a test for each, you will save your time later on when you will revisit
the code to debug or modify. Spend as much time as you can in testing.

Again, this is what I try to stick to. And, lastly, I am not a core
developer, nor a mentor. I am, however, interested in seeing this module
coming alive. So, apologies to the mentors that I am chimingi in.

[rest deleted]

Grazie, Nikos

Hi Nikos!

···

2018-06-04 17:23 GMT+02:00 Nikos Alexandris <nik@nikosalexandris.net>:

Hi all!

Thank you, Moritz and Nikos, for your suggestions!

2018-06-04 11:54 GMT+02:00 Nikos Alexandris <nik@nikosalexandris.net>:

Ciao Roberta,

great work!

Moritz got me faster, as I intended to chime in, with similar
suggestions.

So +1 for:

  • a “real” module, as Moritz described it

Done! I added the folder to my github repository but I have some trouble
compiling the module with g.extension:

g.extension extension=i.sentinel.mask operation=add url=
https://github.com/RobiFag/GRASS_clouds_and_shadows

error message:
g.extension extension=i.sentinel.mask operation=add url=
https://github.com/RobiFag/GRASS_clouds_and_shadows
Fetching <i.sentinel.mask> from <
https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip> (be
patient)…
Compiling…
make[1]: *** No rule to make target /tmp/grass7-roberta-701 4/tmpe7QEso/i.sentinel.mask/scripts/i.sentinel.mask', needed by script’. Stop.
/bin/sh: 1: cannot create /usr/lib/grass74/error.log:
Permission denied
make: *** [i.sentinel.mask] Error 2
ERROR: Compilation failed, sorry. Please check above error messages.

any suggestion?

Maybe irrelevant, but perhaps it is best to name the script as
[i.sentinel.mask.py](http://i.sentinel.mask.py)?

I named the file as you suggested but I still have some problems, now the error message is:

Fetching <i.sentinel.mask> from <https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip> (be patient)…
Compiling…
Installing…
make: *** No rule to make target `install’. Stop.
WARNING: Installation failed, sorry. Please check above error messages.

  • for using the metadata file, and parsing it – my experience with
    Landsat and other high-resolution satellite products, showed it’s best to
    trust the individual metadata that accompany an acquisition.
    Parameters may be very specific to one single acquisition.
    Also, a module that is designed to be “dynamic” is more
    future-proof and easy to maintain and update.
    For example, reading band names from the meta-data, as opposed to more
    “static” designs, i.e. expecting specific hardcoded options, such as
    band names, or their order.

I have already thought about parsing the metadatafile to retrieve the input
bands but at the moment the module requires atmospherically corrected bands
and if users perform the atmospheric correction on their own I don’t have
any control on naming. Moreover, the module needs to know which raster map
corresponds to the blue band, to the green band and so on in order to apply
the rules for clouds and shadows detection. Therefore I need users specify
each band but maybe I can manage this issue requiring a specific suffix in
the bands’ names (e.g. blabla_blue, blabla_green, etc.). In this way, users
have to provide only the prefix (blabla) while the module search for all
raster maps with the specified prefix and at the same time it is able to
recognize each band.

Anyway, the aim of the next coding period is to create a python script that
wraps in a single GRASS module the download and import phase
(i.sentinel.download and i.sentinel.import), the atmospheric correction
using i.atcorr and the cloud and shadow detection procedure. In particular,
for what concerns the atmospheric correction, I’d like to implement an
iterative procedure that executes i.atcorr for all bands of the input image
changing accordingly the requested input parameters. This part should
simplify the management of input maps.

One thing here: massive processing workflows like to have simple
processing building blocks. One Input → one Process → one Output. In
“your” case:

  • Input: name of/request one band and associated
    metadata, and optionally other ancillary data.

  • Process: download, import, removal of atmospheric effects to estimate
    surface reflectance

  • Output: surface reflectance (“corrected” for atmospheric effects) of
    the requested input band

This building block can run in one processing units/node. Making use of many
processing units/nodes, this block can run in parallel for different
input bands.

I suggest to build the module in a way that it runs for one band or as many
bands as the user requires for his workflow. I did this in
https://gitlab.com/NikosAlexandris/i.fusion.hpf/blob/master/i.fusion.hpf.py.
I don’t suggest this script as an example. Yet I think the concept “for
one or many” is useful and wanted.

Thank you! This is very interesting…
I will check your module and I will certainly take it into account even if all the requested input bands are mandatory otherwise the clouds and shadows detection procedures [0] will return wrong results.

As a sidenote:

I think that we don’t use the term “correction”
correctly. An image acquired by a satellite-borne sensor is not “wrong”
to need a correction. Rather, we try to retouch the digital form of the
acquisition, in order to estimate the energy that was actually reflected
by the surface, before the signal travels through the atmosphere.

I studied remote sensing with Thomas Lillesand’s; Ralph W.
Kiefer’s book “Remote Sensing and Image Interpretation”. The book
“correctly” touches this very question.

Maybe it’s only a perspective, and others have another say on this.

  • PEP8 rules and keeping module options in separate lines to increase
    legibility

In line with the suggestions on style, I would stress out to fully
spell out the names of functions, options, descriptions of flags,
iterables and iterators. It will make it easier for the next reader of
your code.

Yes, you are both right!! I have to clean up the code from the style point
of view and it will be one of the tasks of this week and especially of the
last days before the GSoC Evaluation.

Finally, it’s a good idea to write even mini-tests, while you are
progressing, for the small functions that eventually develop to keep the
full scrip compact, functional and more legible.

Writing a test, for your module, in the end, will be much easier.

Sorry Nikos but I don’t really understand what you mean by
“mini-tests”…are you referring to the examples in the manual page?

For a function f(x) that takes x as an input and gives you y as output,
test, for example, whether the output value ‘y’ is within the expected
range. Test your single functions, inside the script. Use assertions, for
example, etc. If you have 10 functions in your script, and have written
a test for each, you will save your time later on when you will revisit
the code to debug or modify. Spend as much time as you can in testing.

Ok, it’s clearer to me now! I will certainly do it!

Again, this is what I try to stick to. And, lastly, I am not a core
developer, nor a mentor. I am, however, interested in seeing this module
coming alive. So, apologies to the mentors that I am chimingi in.

Thanks, I hope you will continue to follow the development of the project/module. As for me also your suggestions and feedback are very useful!

[rest deleted]

Grazie, Nikos

Ciao
Roberta

[0] https://drive.google.com/file/d/1KYEKvNBurBFHw1xUTLjM0PW80Z-7br81/view?usp=sharing

* Roberta Fagandini <robifagandini@gmail.com> [2018-06-04 18:42:47 +0200]:

Done! I added the folder to my github repository but I have some trouble
compiling the module with g.extension:

g.extension extension=i.sentinel.mask operation=add url=
https://github.com/RobiFag/GRASS_clouds_and_shadows

error message:
g.extension extension=i.sentinel.mask operation=add url=
https://github.com/RobiFag/GRASS_clouds_and_shadows
Fetching <i.sentinel.mask> from <
https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip&gt;
(be
patient)...
Compiling...
make[1]: *** No rule to make target `/tmp/grass7-roberta-701
4/tmpe7QEso/i.sentinel.mask/scripts/i.sentinel.mask', needed
by `script'. Stop.
/bin/sh: 1: cannot create /usr/lib/grass74/error.log:
Permission denied
make: *** [i.sentinel.mask] Error 2
ERROR: Compilation failed, sorry. Please check above error messages.

any suggestion?

Maybe irrelevant, but perhaps it is best to name the script as
`i.sentinel.mask.py`?

I named the file as you suggested but I still have some problems, now the
error message is:

Fetching <i.sentinel.mask> from <
https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip&gt; (be
patient)...
Compiling...
Installing...
make: *** No rule to make target `install'. Stop.
WARNING: Installation failed, sorry. Please check above error messages.

It compiles locally. Yet I didn't manage to get it via `g.extension`.
Patch of minor importance attached.

Nikos

(attachments)

0001-Minor-wording-modifications.patch (1.67 KB)

Hi Roberta,

On 04/06/18 16:14, Roberta Fagandini wrote:

2018-06-04 11:54 GMT+02:00 Nikos Alexandris <nik@nikosalexandris.net <mailto:nik@nikosalexandris.net>>:

    - for using the metadata file, and parsing it -- my experience with
      Landsat and other high-resolution satellite products, showed it's
    best to
      trust the individual metadata that accompany an acquisition.
      Parameters may be very specific to one single acquisition.
      Also, a module that is designed to be "dynamic" is more
      future-proof and easy to maintain and update.
      For example, reading band names from the meta-data, as opposed to
    more
      "static" designs, i.e. expecting specific hardcoded options, such as
      band names, or their order.

I have already thought about parsing the metadatafile to retrieve the input bands but at the moment the module requires atmospherically corrected bands and if users perform the atmospheric correction on their own I don't have any control on naming. Moreover, the module needs to know which raster map corresponds to the blue band, to the green band and so on in order to apply the rules for clouds and shadows detection. Therefore I need users specify each band but maybe I can manage this issue requiring a specific suffix in the bands' names (e.g. blabla_blue, blabla_green, etc.). In this way, users have to provide only the prefix (blabla) while the module search for all raster maps with the specified prefix and at the same time it is able to recognize each band.

I forgot that the data had to be pretreated, so reading your argumentation, I actually think your current approach is the most flexible and so the best. Imposing band names is not a good idea IMHO. One option could be a file= parameter which allows providing all the names of all input bands in a text file (respecting a given order), but I think this is not a priority.

Anyway, the aim of the next coding period is to create a python script that wraps in a single GRASS module the download and import phase (i.sentinel.download and i.sentinel.import), the atmospheric correction using i.atcorr and the cloud and shadow detection procedure. In particular, for what concerns the atmospheric correction, I'd like to implement an iterative procedure that executes i.atcorr for all bands of the input image changing accordingly the requested input parameters. This part should simplify the management of input maps.

That's sounds really nice. I agree that your module should do one thing and do it good. As you say, wrapper scripts can then combine different modules to provide more automation.

Moritz

Hi Moritz,

···

2018-06-06 9:12 GMT+02:00 Moritz Lennert <mlennert@club.worldonline.be>:

Hi Roberta,

On 04/06/18 16:14, Roberta Fagandini wrote:

2018-06-04 11:54 GMT+02:00 Nikos Alexandris <nik@nikosalexandris.net mailto:[nik@nikosalexandris.net](mailto:nik@nikosalexandris.net)>:

  • for using the metadata file, and parsing it – my experience with
    Landsat and other high-resolution satellite products, showed it’s
    best to
    trust the individual metadata that accompany an acquisition.
    Parameters may be very specific to one single acquisition.
    Also, a module that is designed to be “dynamic” is more
    future-proof and easy to maintain and update.
    For example, reading band names from the meta-data, as opposed to
    more
    “static” designs, i.e. expecting specific hardcoded options, such as
    band names, or their order.

I have already thought about parsing the metadatafile to retrieve the input bands but at the moment the module requires atmospherically corrected bands and if users perform the atmospheric correction on their own I don’t have any control on naming. Moreover, the module needs to know which raster map corresponds to the blue band, to the green band and so on in order to apply the rules for clouds and shadows detection. Therefore I need users specify each band but maybe I can manage this issue requiring a specific suffix in the bands’ names (e.g. blabla_blue, blabla_green, etc.). In this way, users have to provide only the prefix (blabla) while the module search for all raster maps with the specified prefix and at the same time it is able to recognize each band.

I forgot that the data had to be pretreated, so reading your argumentation, I actually think your current approach is the most flexible and so the best. Imposing band names is not a good idea IMHO. One option could be a file= parameter which allows providing all the names of all input bands in a text file (respecting a given order), but I think this is not a priority.

Thank you for the hint! I think the file option is the best choice at the moment.

Anyway, the aim of the next coding period is to create a python script that wraps in a single GRASS module the download and import phase (i.sentinel.download and i.sentinel.import), the atmospheric correction using i.atcorr and the cloud and shadow detection procedure. In particular, for what concerns the atmospheric correction, I’d like to implement an iterative procedure that executes i.atcorr for all bands of the input image changing accordingly the requested input parameters. This part should simplify the management of input maps.

That’s sounds really nice. I agree that your module should do one thing and do it good. As you say, wrapper scripts can then combine different modules to provide more automation.

Providing more automation as possible is my goal! I hope to be able to reach it :wink:

Roberta

Moritz