add helper method to concatenate vectors

[yyyyx4]

Writing vector(R, list(v) + list(w)) is a common way to concatenate two vectors v and w over some ring R.

In this patch we introduce a simple helper method which allows writing v.concatenate(w) instead.

[RuchitJagodara]

Looks good to me.

[dcoudert]

Have you tried something like

from itertools import chain
return vector(R, chain(self, other))

It might be faster. At least it could be tested.

[mantepse]

Are you sure we want to have this?

[yyyyx4]

@dcoudert Tested it just now for vectors up to length one million; there seems to be essentially no difference in timings.
@mantepse I personally use this operation quite frequently. Why would we not want to have it?

[videlec]

The checks are not enough for other. Having the same base ring is not enough (it could be for example a polynomial).

[mantepse]

Why don't you allow any iterable as other? Put differently: why don't you omit the checks completely?

I think this would be more practical. Is there any danger of having a result that is invalid but would not raise an error?

[videlec]

Why don't you allow any iterable as other? Put differently: why don't you omit the checks completely?

I think this would be more practical. Is there any danger of having a result that is invalid but would not raise an error?

A good reason to have strict specifications is that this could be optimized a lot in subclasses.

[mantepse]

Could you explain how this helps precisely? Or rather, why omitting the checks would make optimisation difficult?

[videlec]

Could you explain how this helps precisely? Or rather, why omitting the checks would make optimisation difficult?

If you know that other is a free module of the same type, you can use Cython to do something like

new_vector = FastVectorConstruction(self._length + other._length)
memcpy(new_vector._data, self._data, self._length * sizeof(mpz_t))
memcpy(new_vector._data + self._data, other._data, other._length * sizeof(mpz_t))
return new_vector

[videlec]

Checking that two vectors have the same parent can be done efficiently, but checking that two vectors only differ by their dimension is trickier. As far as I know, the sage coercion system is not smart enough for that.

[mantepse]

Could you explain how this helps precisely? Or rather, why omitting the checks would make optimisation difficult?

If you know that other is a free module of the same type, you can use Cython to do something like

new_vector = FastVectorConstruction(self._length + other._length)
memcpy(new_vector._data, self._data, self._length * sizeof(mpz_t))
memcpy(new_vector._data + self._data, other._data, other._length * sizeof(mpz_t))
return new_vector

Yes, but couldn't you do the check in the subclasses you wanted to optimise? (Please excuse my possibly naive questions!)

[tscrim]

I agree with @mantepse about having the general case be as general as possible. Subclasses will have to implement their own version for vectors of the same class, which could then punt up to the base class when the type doesn't match.

[yyyyx4]

Thanks everyone for the feedback; I gave it another try.

[github-actions]

Documentation preview for this PR (built with commit 4d1f385; changes) is ready! 🎉

[grhkm21]

It says this branch is out-of-date with the base branch, which I assume means it can be merged without conflicts.

The changes look good to me.