Enhance green-kubo transport and refactor transport module #97
Conversation
- `TransportAutoCorr` is renamed to `TransportKubo` and `ChargeTransport` is renamed to `ChargeDiffusionDynamics` for accuracy. - In `TransportKubo` the current operator are now automatically constructed from `MolList2`. - If Peielrs terms are present in the model `TransportKubo` will split the current operator into two parts and obtain 5 sets of autocorrelation functions in total.
| qutip_res = get_qutip_holstein_kubo(mol_list1, temperature, kubo.evolve_times_array) | ||
| rtol = 5e-2 | ||
| # direct comparison may fail because of different sign | ||
| assert np.allclose(kubo.auto_corr, qutip_res, rtol=rtol) or np.allclose(kubo.auto_corr, -qutip_res, rtol=rtol) |
There was a problem hiding this comment.
Why does the auto-correlation function has a different sign?
There was a problem hiding this comment.
Well, this is embarrassing. At first I thought it's an implementation detail of qutip, but it turned out to be a bug in BraketPair. Going to be fixed in the next commit.
| super().__init__(evolve_config=evolve_config, dump_dir=dump_dir, | ||
| job_name=job_name) | ||
|
|
||
| def _construct_current_operator(self): |
There was a problem hiding this comment.
The model of MolList used here should be general format, right?
There was a problem hiding this comment.
I've prepared vibronic_to_general in mlist.py but forgot to use it. Going to be fixed in the next commit.
renormalizer/transport/kubo.py
Outdated
| symbol1, symbol2 = term[term_idx1].symbol, term[term_idx2].symbol | ||
| if not {symbol1, symbol2} == {r"a^\dagger", "a"}: | ||
| raise ValueError(f"Unknwon symbol: {symbol1}, {symbol2}") | ||
| if symbol1 == r"a^\dagger": | ||
| factor = self.distance_matrix[dof_idx1][dof_idx2] | ||
| else: | ||
| factor = self.distance_matrix[dof_idx2][dof_idx1] |
There was a problem hiding this comment.
The model of BasisMultiElectron will result in an error when constructing the current operator with different input formats.
For example
model[("e_0", )] =["a^\dagger_0 a_1", factor]
model[("e_0", "e_1")] =["a^\dagger_0", "a_1", factor]
model[("e_0", "e_1")] =["a^\dagger", "a", factor]
will give the same Hamiltonian, while only the last format will get a correct current operator.
I think it is ok now, because the transport is currently not mature. At least we should a TODO and doc here to make it clear.
There was a problem hiding this comment.
This is a good point, and I think the origin is that with the complexity of MultiElectron, the key:value structure of model is ill-defined. If the problem is not addressed, it will become difficult for future code to deal with the ambiguity.
When merging MolList and MolList2, I think it's a good chance to improve the interface of MolList2. Let's just make it compulsory to set the index inside the operator ("a^\dagger_0") and use a list instead of a dict to represent the model, and order can be constructed from basis in which each every BasisSet knows the external dofs it represents. I'll elaborate these ideas in another issue.
For this PR I'll check the input to preclude the first example. As for the second example, a ValueError will be raised, which I think is well enough.
There was a problem hiding this comment.
Maybe we could add a function called standardize_model to make the inner format standardized.
There was a problem hiding this comment.
Yes. The two approaches can be combined. I think users will very much prefer model=[["a^\dagger_0", "a_1", factor]] than model[("e_0", "e_1")] =["a^\dagger_0", "a_1", factor] , and imagine constructing MPO by Mpo(mol_list, [["a^\dagger_10", 1]]). The inner standard format checking could do some sorting over list items, detect duplicate terms, convert strings like "e_0" to ("e", 0) tuples, etc.
| @@ -233,21 +247,22 @@ def init_mps(self): | |||
| return BraKetPair(bra_mpdm, ket_mpdm, self.j_oper), BraKetPair(bra_mpdm, ket_mpdm2, self.j_oper2) | |||
|
|
|||
| def process_mps(self, mps): | |||
| # add the negative sign because `self.j_oper` is taken to be real | |||
There was a problem hiding this comment.
The -1 here is due to -1.0j * -1.0j of the two j_operators, right?
If so, why don't keep the -1.0j factor to the j_operator since during the time propagation, the mps is complex anyway.
The -1 here looks strange.
There was a problem hiding this comment.
Yes. j_operator is taken to be real for better performance. In my practice, it is sometimes found that J|ket(0)> is very time and memory consuming. Because |ket(0)> is real, a real j_operator could save half memory and computational cost.
TransportAutoCorris renamed toTransportKuboandChargeTransportis renamed toChargeDiffusionDynamicsfor accuracy.TransportKubothe current operator are now automatically constructed fromMolList2.TransportKubowill split the current operator into two parts and obtain 5 sets of autocorrelation functions in total (see the doc for details).