Fix 2 problems in _multitensor.py

[mhucka]

The dual_basis parameter to the MultiTensor class constructor was defaulted to DualBasis(). In Python, default arguments are evaluated only once at definition time, meaning every MultiTensor instance created without an explicit basis shared the same global DualBasis object. When tests were run in parallel or repeated, data from one test would leak into others, leading to errors.

In addition, MultiTensor.add_dual_elements used list.extend() instead of list.append() when adding DualBasisElement objects. Since DualBasisElement is iterable (yielding its internal terms), using extend() incorrectly decomposed the object into its constituent tuples instead of adding the object as a single unit. This caused MultiTensor.synthesize_dual_basis() to fail in some cases.

[gemini-code-assist]

Code Review

This pull request addresses a bug in the MultiTensor class where a mutable default argument for dual_basis caused shared state between instances. The constructor now correctly initializes a new DualBasis if none is provided. Additionally, the add_dual_elements method was updated to use append instead of extend, and a typo was corrected in the documentation. New test cases verify that MultiTensor instances are isolated and that dual elements are correctly added and synthesized. I have no feedback to provide.

[mpharrigan]

can you update or remove the comment above this.

Are we sure add is the intended operation here?

[mpharrigan]

The function name is plural but the argument name is singular. The docstring says "add" but the inline comment says "extend". maybe this was originally an extend-type operation and was changed to an append-type?

[mhucka]

can you update or remove the comment above this.

Ah, yes, good point. Thanks for catching that. Fixed in the next push.

The function name is plural but the argument name is singular. The docstring says "add" but the inline comment says "extend". maybe this was originally an extend-type operation and was changed to an append-type?

Yeah, something probably happened there. I also think the bug was never caught before now because the tests in _multitensor_test.py were incomplete. (I only ran into the problem because I was working on getting parallel Pytest working, which revealed the problem of initializing MultiTensor with a mutable object, which led to more poking around.)

Here's what's happening:

1. DualBasisElement objects are iterable because they implement the __iter__() magic method (c.f. _dualbasis.py). Each call to that method returns a tuple of 3 items.

2. In Python, doing somelist.extend(someobject) invokes the __iter__ method on someobject. On a DualBasisElement object, doing that ends up adding a tuple of 3 items to somelist. That's in contrast with the operation somelist.append(someobject), which puts the DualBasisElement object itself on somelist.

3. The previous _multitensor_test.py only tested adding an item to the list dual_basis.elements, and never tested using the results. The method synthesize_dual_basis() eventually calls the synthesize_element() method on each item on the list, and therein lies the problem: synthesize_element() does a loop like this:

   for tlabel, velement, coeff in element:

   That works as expected if the values on the elements list in DualBasis are actual DualBasisElement objects because their __iter__() method will get called in the for loop (yielding the 3 items above), but that won't happen if the values on the elements list are tuples.