Opened 17 years ago

Closed 17 years ago

Last modified 16 years ago

#129 closed defect (fixed)

Bug in thinner

Reported by: Dave Offiler Owned by: Dave Offiler
Priority: major Milestone: 1.1
Component: ropp_io Version: 1.1
Keywords: Thinner, interpolation Cc:

Description

Axel reports that v1.1 has a bug in the thinner interpolation code ropp_io_thin_fixed.f90, which was also in v1.0. He had corrected this in his local v1.0 but it had not explicitly been brought it to the attention of the Development Team at the time.

The alleged incorrect code fragment is:

! Search for the pair of source levels just above & below the
! target level, allowing for ascending or descending profile.

        j = 2
        IF ( ascending ) THEN
          DO WHILE ( j < nLev .AND. Lev(j) < ThinLev(k) )
            j = j + 1
          END DO
          i = j - 1
        ELSE
          DO WHILE ( j < nLev-1 .AND. Lev(j) > ThinLev(k) )
            j = j + 1
          END DO
          i = j
          j = j + 1       <<<--- Axel changes to j = j - 1
        END IF

The same change is needed in both LOG (section 2.1)and LIN (Section 2.2).

Change history (3)

comment:1 by Dave Offiler, 17 years ago

Resolution: fixed
Status: newclosed

Rechecking the logic of why I originally (v1.0) thought it ought to be +1 and not -1 : which is that i is the index below the target level and j the one the next one above (ie higher). For descending profiles, the WHILE loop terminates when j is the lev below thinlev; set i to that index, then set j to be the index next *higher* level. The bug here is in still thinking 'next index' means 'higher level' - which for a descending occultation is the wrong direction. So it should indeed be -1.

The bug is generic and potentially affect all parameters, and all interpolation thinning methods.

Ran the ropp_io/tests/t_ropp_thin.sh thinner test to generate graphical outputs of thin-original differences from the j+1 version of ropp2ropp (currently at v1.1.1). Save jpg files.

Edited the two changes (to j-1), run make and rerun the above test script.

Comparing all the tested thinner option outputs:

1) NONE: no effect (as expected)

2) SAMPLE : no effect (as expected)

3) A/SG/LOG/LIN 64 : slight improvement in noisy BA at lowest levels. Ref bias reduced at lowest levels.

4) A/SG/LOG/LIN 247 : Largest spikes at low level removed. Small Ref bias at lowest level reduced to within general variability.

Since low level spikes (in BA) or bias (in Ref) have been removed or significantly reduced, and no-where worse, it is judged that the change is both correct and beneficial.

The noise in BA at very low levels was present at v1.0, and was attributed to the test procedure, which re-interpolates thinned values back onto the original levels, which would naturally cause noise where there is significant structure in the hi-res data lost by the thinning process (as still may be the case).

This bug would be most evident only in areas of extreme vertical gradient - as for BA below ~3-4km in GRAS data - and be manifested as a bias in the opposite direction to the gradient (+ve bias for large -ve gradient). This can explain at least some of the differences Sean sees, but we'll have to wait for data to tell if it's the whole story or some residual bias remains.

Martin & Axel informed of the change and resulting code designated v1.1.2.

Committed as [1523].

comment:2 by (none), 16 years ago

Milestone: 1.1

Milestone 1.1 deleted

comment:3 by Huw Lewis, 16 years ago

Milestone: 1.1
Note: See TracTickets for help on using tickets.