Wish list

The caller must pass ownership of other. Thus, anyone who wants to use this method must clone other, which seems wasteful. Instead the type of other should be &Self. But this item is obsoleted by the next item...
The only use of other in this implementation is to call other.size(), which merely returns a usize struct field. Thus, there is no need for an already-constructed EvaluationDomain for other here. Indeed, I suspect a common use of this method is where a caller knows the size of other but hasn't actually constructed a EvaluationDomain for it. In those use cases this method forces the caller to construct an EvaluationDomain just to call this method. I suggest changing the type of other from Self to usize. In that case it's name should be changed to something like other_domain_size.

Code comments are hard for me to understand. Seems like the rustdoc for this method should read something like

/// Convert `index` from [an index into an element from `other`] into [an index of an element from `self`].
///
/// `other` must be a sub-domain of `self`.
/// etc.

The following comment https://github.com/arkworks-rs/algebra/blob/273bf2130420904cab815544664a539f049d0494/poly/src/domain/mod.rs#L289-L292 can be replaced with something shorter:
```
// `other` is a subgroup of `self`. Thus, index `i` in `other` is index `i*|self|/|other|` in `self.
```
I don't understand the following comment https://github.com/arkworks-rs/algebra/blob/273bf2130420904cab815544664a539f049d0494/poly/src/domain/mod.rs#L297-L304 It's not clear to me why this method should allow index >= other.size(). Shouldn't this be an out-of-bounds error?

arkworks-rs / algebra

EvaluationDomain::reindex_by_subdomain ergonomics and doc improvements #853

Wish list