Switch Tensor to use ArcArray instead of Array

[ihincks]

The motivation for this change in what's stored by a tensor is to allow a cheap cloning mechanism that, in particular, a QuantumProgram can exploit. Since some arrays it deals with may be large, we don't want to force it to clone entire arrays when it needs to do things like fan out a tensor to multiple descendent nodes when evaluating the graph.

See this one commit for the diff being introduced here: 257cd56

PR Stack

• Parent: Refactor ProgramNode::call to flatten+call_flat+unflatten #16298
• Compare to parent
• Full Stack

AI/LLM disclosure

• I didn't use LLM tooling, or only used it privately.
• I used the following tool to help write this PR description:
• I used the following tool to generate or modify code: Claude 4.7

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core

[coveralls]

Coverage Report for CI Build 26910624413

Coverage decreased (-0.006%) to 87.465%

Details

• Coverage decreased (-0.006%) from the base build.
• Patch coverage: 3 uncovered changes across 1 file (68 of 71 lines covered, 95.77%).
• 12 coverage regressions across 6 files.

Uncovered Changes

File	Changed	Covered	%
crates/providers/src/tensor.rs	68	65	95.59%
Total (2 files)	71	68	95.77%

Coverage Regressions

12 previously-covered lines in 6 files lost coverage.

File	Lines Losing Coverage	Coverage
crates/qasm2/src/parse.rs	6	97.63%
crates/providers/src/tensor.rs	2	94.8%
crates/circuit/src/parameter/parameter_expression.rs	1	91.03%
crates/circuit/src/parameter/symbol_expr.rs	1	73.88%
crates/qasm2/src/expr.rs	1	93.82%
crates/qasm2/src/lex.rs	1	92.29%

---

Coverage Stats

Relevant Lines:	124528
Covered Lines:	108918
Line Coverage:	87.46%
Coverage Strength:	961673.64 hits per line

---

💛 - Coveralls

[mtreinish]

I'm onboard with moving to ArcArray here, but I just have one comment on the ndarray API usage. I think a lot of the into_shared() (if not all of them, I didn't check every individual call) would be better suited as an .into() to move the owned array's data into the arc array instead of copying it.

[mtreinish]

Does this need to be into_shared(), I think we could use into() instead? Looking at the docs on this method: https://docs.rs/ndarray/latest/ndarray/type.ArcArray.html#method.into_shared it seems to indicate it will potentially clone the data again. We just made a new copy with mapv() so we should be able to just run .into() to just move it into an Arc. This applies a bunch of places I think, basically anywhere we construct a new owned array for the sole purpose of creating an ArcArray where we don't need to preserve the the original owned array.

[ihincks]

Since into_shared() is part of BaseArray, it needs to cover Array, ArcArray, and CowArray. The only place where it clones the underlying data is when called on a CowArray that does not own its data. This is true even if the data is strided or otherwise not contiguous in an array view.

The docs do recommend using into() instead of into_shared() when writing a function that's generic over ArcArray and Array: this is because into_shared demands Clone (because of the CowArray implementation) whereas into does not require Clone in this case when going into an ArcArray.

This is all to say: I'm pretty sure all of the into_shared in this PR are fine and can't cause data copies. I just checked, and all of them can be turned into into(). So I think this comes down to style. Do we want to prefer into() when the type inference is available instead of into_shared() as a blanket convention so that we're a bit more on our toes about the CowArray copy case?

[mtreinish]

Yep, you're right. Looking deeper at the ndarray code, the OwnedArray DataOwned::into_shared implementation just used ArcArray::from internally:

https://github.com/rust-ndarray/ndarray/blob/27af864668320e9092a20fa29dee753123798084/src/data_traits.rs#L541-L557

so it's doing the same thing as into(). I prefer into_shared() as I think it's more clear (just as I had that nit comment about to_vec() on the previous PR), I only left the comment because of the docs warning. Lets leave it as is.

I think it's unlikely we'll deal with a lot of CowArray objects if we're using ArcArray already here but it's good to cover that use case just in case.

[mtreinish]

Before I enqueue I just had one small question

[mtreinish]

Actually why do we need this type? Isn't it in ndarray already? https://docs.rs/ndarray/latest/ndarray/type.ArcArrayD.html

[ihincks]

Ah, looks like it was introduced in ndarary 0.17, but we're on 0.16. Can I upgrade to that version?

rust-ndarray/ndarray#1561

[mtreinish]

We're stuck on 0.16 for right now because of rustworkx-core IIRC. We get arrays from rustworkx-core (like adjacency and distance matrices) so the version we use internally for Qiskit has to match the version rustworkx-core uses which is unfortunately pinned to 0.16 right now. It's something we want to fix for the next rustworkx-core release