before embedding check that the destination basis matches the operator basis

[atombear]

closes #240

[david-pl]

I changed the syntax as discussed in #240. I'm sure things could be more elegant, but at least everything seems to work properly like this. Let me know if you have any suggestions before I merge this.

[coveralls]

Coverage decreased (-0.02%) to 96.677% when pulling 82daf53 on atombear:bug/embed_checks_bases into 40d0231 on qojulia:master.

[codecov]

Codecov Report

Merging #246 into master will increase coverage by 3.24%.
The diff coverage is 96%.

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
+ Coverage    93.4%   96.65%   +3.24%     
==========================================
  Files          36       36              
  Lines        2973     2897      -76     
==========================================
+ Hits         2777     2800      +23     
+ Misses        196       97      -99

Impacted Files	Coverage Δ
src/operators_sparse.jl	84.52% <0%> (-3.14%)	⬇️
src/operators.jl	97.72% <100%> (+2.15%)	⬆️
src/operators_lazysum.jl	89.74% <0%> (-7.48%)	⬇️
src/bases.jl	94.66% <0%> (-1.28%)	⬇️
src/fock.jl	100% <0%> (ø)	⬆️
src/manybody.jl	100% <0%> (+0.9%)	⬆️
src/operators_lazytensor.jl	100% <0%> (+1.12%)	⬆️
src/stochastic_schroedinger.jl	100% <0%> (+2.17%)	⬆️
... and 16 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07db21f...60321e0. Read the comment docs.

[codecov]

Codecov Report

Merging #246 into master will decrease coverage by 0.01%.
The diff coverage is 97.87%.

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
- Coverage   96.69%   96.67%   -0.02%     
==========================================
  Files          36       36              
  Lines        2874     2919      +45     
==========================================
+ Hits         2779     2822      +43     
- Misses         95       97       +2

Impacted Files	Coverage Δ
src/operators_sparse.jl	84.52% <0%> (-1.02%)	⬇️
src/sortedindices.jl	100% <100%> (ø)	⬆️
src/operators.jl	97.94% <100%> (-0.24%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40d0231...82daf53. Read the comment docs.

[atombear]

i added a method that performs the index wrangling to embed an operator in a joint hilbert space as defined by a subset of the bases. i'm not sure if this is redundant with the change you added, but i'll work to understand it better.

[david-pl]

I didn't go through your code in detail, so correct me if I'm wrong, but it seems like your function handles the case of a single (composite) operator being embedded in a product basis only. This case is covered by the method I wrote as well. So yes, I think this is a bit redundant.

[atombear]

Yes, my method handles the case of one operator being embedded in any subspace of a composite hilbert space. so, for 8 qubits, D=2^8, and a CNOT will be embedded appropriately in any 4d supspace, meaning in any 2 indices. I think your method does not distinguish between an operator acting an different, but equivalent, sub-bases. for example, for a 5x3 operator being embedded in a space that is 5x3x3, your method gives the same output whether the indices are [1, 2] or [1, 3], but those should yield different operators.

[atombear]

so i think my position is as follows: the original code (indices, ops) tried to match every op to an index, but had the problem that the matching would not bother checking that the op matched the basis corresponding to the index in which it was being inserted.

the new method (indices, ops) tries to do two things:

• assert that the size of the basis matches, thereby fixing the bug
• try to account for composite spaces by index counting

however, there is an oversight in that it assumes operators in composite spaces correspond to consecutive basis indices. i think it is impossible to correct for this without the index wrangling. Additionally, it assumes the following syntax: [1,2,3], [op12, op3] to place a composite operator op12 in the space [1,2], and a third operator op3 in space 3. perhaps the syntax [[1,2],3], [op12, op3] would be clearer?

the new (indices, op) method generically embeds an operator in a composite hilbert space in an even larger hilbert space, by doing the appropriate index wrangling.

i think it is true that the more general and bug-free (indices, ops) method can be written in terms of the new (indices, op) method, allowing for non-consecutive indices. my proposal is to do this, and i am glad to take on the task if that makes sense to you. additionally, i am happy with any syntax, though the second i think is clearer, eg: [[1,3],2], [op13, op2].

[atombear]

i think i have pushed a test-passing working version of what i described. there are a few more tests i'd like to add, but i think the code is at least correct (finally). a review would be much appreciated at this point.

[david-pl]

I appreciate all your work. The changes look good and I agree with the new syntax, which makes a lot more sense especially in the case that I oversaw (embedding composite operators on bases such as e.g. [1,3]). I'm also not sure why the tests are failing. Hopefully it was just a hick-up, so I triggered them by closing/re-opening.