dsharlet
`array` and `array_ref` `operator==` and `operator!=` don't short circuit
Oof, I just ran into this.
Not only do they not short circuit, they compare all elements, which can be slow and surprising without looking at the code.
I expected built-in operations on array_refs to be cheap, with slow operations deferred to the free functions nda::equal, nda::make_copy, etc. And we intentionally made it so that array does not have a copy constructor from array_ref.
@jiawen it sounds like you expect this to be a shallow equality? This issue is about a much more subtle issue: when doing a deep equality check, we don't short circuit upon finding a falsifying element.
Re: deep vs. shallow equality: I made array::operator== deep quality because that's what std::vector::operator== does: https://en.cppreference.com/w/cpp/container/vector/operator_cmp
Maybe it's reasonable to make array_ref::operator== a shallow equality check, while keeping array's the same for std::vector similarity?
Oops. It was nearby but that TODO in the code didn't actually refer here.
Re: operator==, I think of nda::array <--> std::vector and nda::array_ref <--> std::span. For the former, I agree that the behavior should match. By the same token, maybe we should remove nda::array_ref::operator== and add something like nda::ptr_equals or the more cumbersome nda::ptr_and_shape_equals.
Notably, absl::Span talks about how unlike std::span, it provides operator== and is likely a design mistake.
To not break existing users, maybe add #define NDA_INCOMPATIBLE_DEPRECATED_ARRAY_REF_OP_EQUALS that defaults to false (but can be flipped back on). And like other projects, remove the option after a few years (or never).
I agree we should probably remove array_ref::operator==.
Maybe we just don't need to provide a helper for this? If it's just a.base() == b.base() && a.shape() == b.shape(). Looks like std::span doesn't provide anything like this either.
I think we should try just removing it. I don't think the usage of this library is big enough to worry about maintaining compatibility for such a likely obscure use case. We already have nda::equal so replacing operator== usage with that call is pretty easy.
