Fix replace! function when replacing a Tensor with itself in pair argument

[jofrevalles]

Summary

This PR resolves #225 by fixing the problem with replace! when the same Tensor appears in both positions of the pair argument. Previously, since we did not have any specific check for this case, the replace! function had a push! that did not add the tensor since it was already present, but then, because we had a delete! after the push!, the Tensor got deleted.

We fixed this problem by adding a condition in the replace! function that checks if the two tensors are equal, and if they are, the function returns the TensorNetwork without changes.

Additionally (and importantly @bsc-quantic/software !), this PR also fixes the commented-out tests for the replace! function, and we have extended these tests to cover the specific case that this PR addresses.

[mofeing]

mmm shouldn't this be === to check for egality, not equality?

Suggested change

	old_tensor == new_tensor && return tn
	old_tensor === new_tensor && return tn

[jofrevalles]

No! Since for example, now it failed when we did replace!(tn, A => conj(A)), when A is a Tensor with a parent Array that are Real.

[mofeing]

Exactly! If eltype(A) <: Real, then A === conj(A)!

[jofrevalles]

conj(A) returns a new object with is equal to A. It would be like you say if it is conj! (not conj).

[mofeing]

You're right with this last point, but the bug in #225 is when it's replaced with the same tensor (i.e. old_tensor === new_tensor). new_tensor = copy(old_tensor) should work and there's nothing bad with it even if it really doesn't change the TN.

I try to avoid == if posible because == iterates over the whole arrays and checks for equality element-wise, while === just checks the objectids and you're done. Thus it avoids the overhead of iterating over everything, which can be costly on large arrays (specially when we start using distributed arrays).

[jofrevalles]

but the bug in #225 is when it's replaced with the same tensor (i.e. old_tensor === new_tensor)

Yes, but I tried replace!(tn, A => conj(A)) and we got also the same error. I think this is because if you do conj(A) you still get the same objectid, which for me this is strange:

julia> objectid(A)
0xc7798e05eba779fe

julia> objectid(conj(A))
0xc7798e05eba779fe

But whatever, okay! If that is the case I agree that we can have ===, I will push that commit now.

[mofeing]

Awesome! I'm leaving one import thing (egality vs equality) and some grammatical fix requests.

[jofrevalles]

@mofeing I am merging this PR on monday. Let's enjoy the weekend :)