Symbolic Hamiltonian with constants #1389
Conversation
…ians containing constants Cf. #1383
|
@mho291 I started adding a minimal test that reproduces your issue. I'm now going to add the patch that should make that test passing. Whenever you have time, confirm that the test is close enough to what you expect (then I will also ask to check the full fix). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1389 +/- ##
==========================================
- Coverage 99.84% 99.84% -0.01%
==========================================
Files 76 76
Lines 10844 10843 -1
==========================================
- Hits 10827 10826 -1
Misses 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
For the time being, it's a quick patch (plus some docstrings beautifications). I wanted to refactor a bit more the function. But I took a look at the rest of the module, and there are many things to refactor. So, it's not worth to do it now, better to allocate some time and do it consistently. |
| @@ -566,8 +568,8 @@ def expectation_from_samples(self, freq, qubit_map=None): | |||
| if subk.count(1) % 2 == 1: | |||
| expval_k = -1 | |||
| expval_q += expval_k * counts[i] | |||
| expval += expval_q * float(coeff[j]) | |||
| return expval | |||
| expval += expval_q * self.terms[j].coefficient.real | |||
There was a problem hiding this comment.
Any clue why SymbolicTerm coefficient are stored as complex values?
qibo/src/qibo/hamiltonians/terms.py
Line 138 in c3031c6
O.e. I believe they are terms in a Hamiltonian, that is hermitian, and I imagine in general a superposition of other Hermitian terms, with real coefficients.
Though you could also have anti-hermitian with imaginary coefficients, or sums with the Hermitian conjugate. So, maybe it's just here that, since terms made of only Z are required, the coefficients are expected to be real...
There was a problem hiding this comment.
I think terms here may not necessarily be hermitian. A term is a product of symbols and it happens that the symbols we provide are the Paulis which are all hermitian, however the user can also define their own symbol using custom matrix. So there may be a case where you have non-hermitian terms and complex coefficients resulting to a hermitian Hamiltonian.
The constant, on the other hand, probably needs to be real in every case, since it is multiplying the idenity that is hermitian.
|
Thanks @alecandido ! I have checked the code and verified it works. Will approve the changes. |
Fixes #1383
Checklist: