[GRASS-dev] [GRASS GIS] #2918: Infinite loop in i.landsat.toar with method=DOS4

#2918: Infinite loop in i.landsat.toar with method=DOS4
---------------------------+-------------------------
Reporter: DmitryKolesov | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.4
Component: Imagery | Version: unspecified
Keywords: | CPU: Unspecified
Platform: All |
---------------------------+-------------------------
== Description: ==
I.landsat.toar with method=DOS4 fails to stop. For example if run the
module using Landsat8 scene (id=LC81680222013351LGN00) as input, the
module doesn't finish the calculation.

== Details: ==
The problem cycle is the loop on lines 109--115
(http://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/imagery/i.landsat.toar/landsat.c#L109).
This is an iterative process for approximate atmospheric transmittance
coefficients TAUv and TAUz.

Math details of the approximation are described in <<Song, C., Woodcock,
C. E., Seto, K. C., Lenney, M. P., & Macomber, S. A. (2001).
Classification and change detection using Landsat TM data: when and how to
correct atmospheric effects?. Remote sensing of Environment, 75(2),
230-244.>> (https://scholar.google.ru/scholar?cluster=2327996098064583125,
see equations 5 -- 10).

But the process can't stabilize if solar elevation angle is small (for
example for winter scenes in North semisphere): in this case Tz (line 113,
http://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/imagery/i.landsat.toar/landsat.c#L113)
is negative because of division:

{{{
Tz = 1 - (4. * pi_d2 * Lp) / (lsat->band[i].esun * sin_e)
}}}

There pi_d2, lsat->band[i].esun are constants, Lp is finite positive
number, but sin_e can be very small (if solar elevation angle is small).
As result, Tz is negative, log(Tz) is NaN
(http://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/imagery/i.landsat.toar/landsat.c#L114),
so Tv is Nan and the exit condition isn't reachable.

== Proposed solution: ==
I see two possibilities if Tz becomes NaN:
  * emit fatal-error like "DOS4 method isn't applicable for the scene. Use
other method or use other sun_elevation parameter." and exit.
  * emit warning, then reset Tz and Tv, using default values (1.0) for the
coefficients
(http://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/imagery/i.landsat.toar/landsat.c#L122).

I think I could write a patch, but I need an advise about solution.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2918&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.4
Component: Imagery | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------

Comment (by mlennert):

Replying to [ticket:2918 DmitryKolesov]:
> == Description: ==
> I.landsat.toar with method=DOS4 fails to stop. For example if run the
module using Landsat8 scene (id=LC81680222013351LGN00) as input, the
module doesn't finish the calculation.

Thank you for the very detailed description and the nice catch !

[...]

> == Proposed solution: ==
> I see two possibilities if Tz becomes NaN:
> * emit fatal-error like "DOS4 method isn't applicable for the scene.
Use other method or use other sun_elevation parameter." and exit.
> * emit warning, then reset Tz and Tv, using default values (1.0) for
the coefficients
(http://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/imagery/i.landsat.toar/landsat.c#L122).
>

I'm no expert in the different DOS versions, but DOS4 is defined as:

DOS4: TAUv = exp[-t/cos(sat_zenith)], TAUz = exp[-t/sin(e)], Esky = PI *
radiance_dark

Does this really make sens when TAUv and TAUz are set to 1 ?

Unless it does, I would prefer your first solution.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2918#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.5
Component: Imagery | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------
Changes (by DmitryKolesov):

* Attachment "i.landsat.toar.landsat.c.diff" added.

Patch diff

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2918&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.5
Component: Imagery | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------

Comment (by DmitryKolesov):

Please review the patch (see the attachment). I emit fatal-error in case
of unstable approximation.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2918#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.0.6
Component: Imagery | Version: unspecified
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------
Changes (by neteler):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"69793" 69793]:
{{{
#!CommitTicketReference repository="" revision="69793"
i.landsat.toar: avoid endless loop and exit if approximation of
atmospheric transmittance coefficients is unstable (DOS4 method)
(contributed by Dmitry Kolesov, fix #2918)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2918#comment:5&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.0.6
Component: Imagery | Version: unspecified
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------

Comment (by neteler):

(from offlist communication)

On Tue, Nov 8, 2016 at 1:43 PM, E. Jorge Tizado wrote:
> I prefer the first option: emit fatal-error like "DOS4 method isn't
> applicable for the scene because Because of not convergence of the
> method. Use other method or use other sun_elevation parameter." and
exit.
>
> I think that if it is not possible to use the DOS4 method, the user must
> know it and not generate an output different to the requested DOS4.
> Then, the user has to use consciously another atmospheric correction
method.

I have submitted the patch (slightly reworded) and backported it to 7.2
and 7.0.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2918#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.0.6
Component: Imagery | Version: unspecified
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------

Comment (by neteler):

In [changeset:"69794" 69794]:
{{{
#!CommitTicketReference repository="" revision="69794"
i.landsat.toar: avoid endless loop and exit if approximation of
atmospheric transmittance coefficients is unstable (DOS4 method)
(contributed by Dmitry Kolesov, fix #2918)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2918#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#2918: Infinite loop in i.landsat.toar with method=DOS4
----------------------------+-------------------------
  Reporter: DmitryKolesov | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.0.6
Component: Imagery | Version: unspecified
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: All
----------------------------+-------------------------

Comment (by neteler):

In [changeset:"69795" 69795]:
{{{
#!CommitTicketReference repository="" revision="69795"
i.landsat.toar: avoid endless loop and exit if approximation of
atmospheric transmittance coefficients is unstable (DOS4 method)
(contributed by Dmitry Kolesov, fix #2918)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2918#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>