Clear read/write event for the global matrix_cls

[rok-cesnovar]

We should add a matrix.wait_for_read_write_events() for all data matrix_cls in the log_prob_impl.

Right now, a kernel call to a data matrix adds a read event to the read event vector: https://github.com/stan-dev/math/blob/develop/stan/math/opencl/matrix_cl.hpp#L50
These are never cleared between log_prob_impl calls which means the events add on, causing a rising memory consumption over time.

Adding matrix.wait_for_read_write_events() for all data matrices in log_prob_impl will clear these vectors.

[SteveBronder]

@t4c1 could we add a template like bool AsyncTrack = true that make the read and write event setters into no-ops? Then in the compiler Rok can just set data to have a false value for the matrix_cl data

[t4c1]

Then in the compiler Rok can just set data to have a false value for the matrix_cl data

I don't understand this second part of the suggestion.

But not setting events will cause wrong results on any async platform unless we set async flag to false.

[SteveBronder]

So if we had something like

template <typename T, bool TrackEvents = true>
class matrix_cl<T, TrackEvents, require_arithmetic_t<T>> : public matrix_cl_base

Then for add_read/write_event we could just do

inline void add_read_event(cl::Event new_event) const {
   if (TrackEvents) {
    this->read_events_.push_back(new_event);
   }
}

So in the model constructor we would make

matrix_cl<double, false> X_opencl_ // ...

We know the data in a model is immutable so we don't need to track write events (though we should wait for the first transfer) and idt we care about keeping track of where they are being read from do we?

[t4c1]

That seems a bit complicated and dangerous. If we ever write to such a matrix we will likely get wrong results somewhere. Regularly cleaning up events by calling .wait_for_read_write_events() seems both simpler and safer.

[rok-cesnovar]

I agree with Tadej that this adding a few waits is simpler.

[SteveBronder]

I'm cool with whatever works! If the data is used in the reverse pass should we do this by adding a reverse_pass_callback() at the top of log_prob that calls wait_for_read_write_events() for each matrix of data?

[t4c1]

No need to complicate. We just need wait_for_read_write_events() called regularly. It does not matter if it is forward or reverse pass (or even if it is in every log_prob_impl call). Calling it once at the start or end of log_prob_impl is just the simplest option.