Repository navigation
and_broadcast_mut for Zip #478
Description
Activity
Broadcasting isn't compatible with mutable access in general. We can use a Vec or 1D array as a model for this. We assume throughout the code that if the element index is different, two mutable references to elements are not aliasing, so we can modify though both paths. This breaks if we have mutable broadcasting.
It could only be allowed if we imposed something like the access pattern of a
Cell, and not allowing mutable references.Which brings a "solution": If an
ArrayViewMut<T>can be converted into anArrayView<Cell<T>>elements, which is a transformation that has been discussed before and we have a similar example here in libstd https://doc.rust-lang.org/nightly/std/cell/struct.Cell.html#method.as_slice_of_cells.In short: .and_broadcast_cell ? Or General ArrayViewMut -> ArrayView of Cells?
One cool fact is that the non-parallelization should work itself out automatically by the Send/Sync traits; when the Zip contains a Cell, the compiler will disallow thread parallelism.
I think
ArrayViewMut<T>borrowed as aArrayView<Cell<T>>looks like quite a nice solution, and might even come in handy elsewhere. From this it seems simple, are there any pitfalls in extending this to casting the backing array? I'll give implementing this a go.We should be able to start with just an
ArrayViewMut::as_cell_view, or whatever name feels best, method and try to plug it in the existing broadcast code. Ergonomics of Cells and value updates are of course not ideal.I guess there is a chance these methods are being held back for technical or "unsafe code guidelines/formalization" reasons upstream. I can't spot any discussion along those lines though. The
Cellhas this attribute in Rust now, which is a stable attribute too:#[repr(transparent)].. o O ( So what if there's an arithmetic compatible transparent wrapper type around Cell )
The
Cellhas this attribute in Rust now, which is a stable attribute too:#[repr(transparent)].#[repr(transparent)](at least onCell) is in beta, but not stable yet. (It should be released as stable in a couple of weeks.) (stable docs, beta docs)So what if there's an arithmetic compatible transparent wrapper type around Cell
I think the
#[repr(transparent)]annotation is technically relevant for our use-case because AFAIK Rust doesn't guarantee any particular bitwise representation by default except for fixed-size arrays. (SoCellneeds to have#[repr(C)]or#[repr(transparent)]for us to be able to safely cast theArrayViewMutin-place.) In practice, though, I don't think we have to worry too much about this becauseCell<T>is just a thin wrapper aroundUnsafeCell<T>which is just a thin wrapper aroundT. Regardless, it'll be resolved in a couple of weeks anyway.I wonder, after we start supporting Cell there will be a want for supporting something that wraps Cell (with the same features) that also supports arithmetic. So that it can participate in operations like
Array1<f32> + ArrayView1<MathCell<f32>>
Currently Zip has no mutable equivalent for the
and_broadcastmethod, which is presumably to allow it to safely besplit()and parallelized. However this makes some calculations difficult. The examples below are for backprop through a broadcast-able element-wise multiplication, but the problem appears as a pattern often.Currently I'm looking at using this:
which works correctly, but also has UB written all over it depending on how Zip is implemented internally (I realize Zip owes me nothing here).
The safe alternative I've found is:
which is catastrophically (over 30x) slower for some cases.
So my questions are:
and_broadcast_mut()method which would set a flag or something which makessplit()panic?