Why do we store the visited cells...?

[Datseris]

cc @awage

Why does BasinInfo contain a field visited_list::Vector{CartesianIndex{D}}? Is this used anywhere? I am reading the source code but I don't understand if it is used anywhere.

THe only point in the source code that .visited_list is populated with cells is here:

    if bsn_nfo.state == :att_search
        if ic_label == 0
            # unlabeled box, label it with current odd label and reset counter
            bsn_nfo.basins[n] = bsn_nfo.visited_cell_label
            push!(bsn_nfo.visited_list, n) # keep track of visited cells
            bsn_nfo.consecutive_match = 1
        elseif ic_label == bsn_nfo.visited_cell_label
            # hit a previously visited box with the current label, possible attractor?
            bsn_nfo.consecutive_match += 1
        end

There are only 2 sections in the source code where .visited_list is called, and they are both the same function call:

relabel_visited_cell!(bsn_nfo, bsn_nfo.visited_cell_label, 0)

which calls the function:

function relabel_visited_cell!(bsn_nfo::BasinsInfo, old_label, new_label)
    while !isempty(bsn_nfo.visited_list)
        ind = pop!(bsn_nfo.visited_list)
        if bsn_nfo.basins[ind] == old_label
            bsn_nfo.basins[ind] = new_label
        end
    end
end

There are two things I notice:

1. old_label can never be the same as new_label, as the visited cells by definition have label at least 4, while new_label is always 0 given how we call the function.
2. We do not use the list of visited cells. Like, wow. The way we check if we have entered a visited cell is by checking the cell label that is stored in the basins array.

So, @awage, do we actually need the visited_list ? I don't see how. Seems to me like we are wasting computations by storing, and then removing the visited cells, without actually using them anywhere!

[Datseris]

Okay, in PR #101 I've removed the storage of visited cells, but all tests went to hell. So maybe we use them somewhere? But I can't tell where.

[Datseris]

(hence, one more reason for PR #101 . The source code needs to clarify these things)

[awage]

Hi! This is a queue to optimize the cleaning of the basins matrix. Once the algorithm is finished, all the cells that have not been labeled as a basin or as an attractor are wipped from the matrix. Instead of searching for these labels in the matrix you store the visited cells in a queue and set these entries to zero.

This optimization is crucial for the speed of the algorithm. But there are surely better ways to name these functions, something like basins_cleanup.

[Datseris]

Aaaaaaaah now I understand.... Okay, I'll put this as a comment in #101 because I couldn't understand this.