Bug in Cross-contour_transport.ipynb

[schmidt-christina]

When adding the two additional options during the last update, we only adjusted mask_x_transport but not mask_x_transport_numbered and the same problem for the other case where we changed mask_y_transport (not shown):

    # if point to left and right BOTH toward Antarctica
    elif (contour_masked_above_halo[index_j, index_i]==0) and (contour_masked_above_halo[index_j, index_i+2]==0):
        mask_x_transport[index_j, index_i-1] = 1
        mask_x_transport[index_j, index_i] = -1        
        mask_x_transport_numbered[index_j, index_i-1] = new_number_count
        new_number_count += 1

My fix:

    # if point to left and right BOTH toward Antarctica
    elif (contour_masked_above_halo[index_j, index_i]==0) and (contour_masked_above_halo[index_j, index_i+2]==0):
        mask_x_transport[index_j, index_i-1] = 1
        mask_x_transport[index_j, index_i] = -1        
        mask_x_transport_numbered[index_j, index_i-1] = new_number_count
        mask_x_transport_numbered[index_j, index_i] = new_number_count+1
        new_number_count += 2

A first test confirmed that my addition fixes the bug which resulted in the cross contour transport being really different with the current version, especially across the 2500 m isobath.
Here, I compare the circumpolar cumulative transport summed up over dense layers in year 2000 using the different methods to create the isobath

• old: before we added the new two options
• with bug: with the new options but a wrong numbered mask, so that we only added one of the two added gridcells
• new: with the bug fixed as suggested above

For the 1000m isobath, the results between old and new are fairly similar, so that’s good. For the 2500 m isobath, the bug really changes the result but also the old and new version are not as similar as I expected. But I do think my fix is correct and it indicates that the additional two options have a greater impact than I realised.

[claireyung]

Hey Christina,

Thanks so much for finding the bug and making this fix!

I just have a small problem/question - should we expect the contour number to be monotonically increasing along the contour? At the moment, the two ones that are set at the same time by the new edits are adjacent numbers, whereas if you were following the contour you'd expect their numbers to be 2 apart with the other coordinate in between.

E.g. for this contour:

the numbers of mask_y_transport_numbered and the same for x are below, note that the y direction is reversed when I printed it out (with Antarctica at the top, sorry) compared to the plots. Note the x ones are adjacent integers (4397,4398). I think they should actually be 4396 and 4398 and the jutting out y mask should be 4397

Also, for this contour (which I don't think was modified from the changes in #276 for issue #265) the numbers are three apart (3025,3028). This is I suppose another incompatibility of the original code with the contour that has diagonals.

I suspect this won't affect which numbers get picked up by the mask algorithm, i.e. we should get correct total transports with your fix and it's much better than when we were completely getting wrong transports due to the misnumbering (sorry for my ignorance of that!!). However I think that the numbering order is not quite right so we plot along-contour transports so a few things will be a little jumbled. Probably worth fixing, but I guess this will be going through more if-and-else-if cases....

Thoughts?