Feature/qed couplings#115
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #115 +/- ##
===========================================
+ Coverage 95.42% 95.61% +0.19%
===========================================
Files 72 73 +1
Lines 3301 3445 +144
===========================================
+ Hits 3150 3294 +144
Misses 151 151
Flags with carried forward coverage won't be shown. Click here to find out more.
|
alecandido
left a comment
alecandido
left a comment
There was a problem hiding this comment.
I reviewed everything but the actual couplings.py file and compatibility.py, I'll do as soon as possible.
|
I didn't review line by line the whole If you can fix the last two comments, I believe you can merge right after. |
|
@felixhekhorn @alecandido Now everything should be fixed. Can we merge? |
| new_theory["alphaem"] = new_theory.pop("alphaqed") | ||
| if "QED" in new_theory: | ||
| new_theory["order"] = (new_theory.pop("PTO") + 1, new_theory.pop("QED")) | ||
| if operators is not None: |
There was a problem hiding this comment.
It is a bit asymmetric that you allow for operators to be None, but not theory.
There was a problem hiding this comment.
The best would be to have two decoupled methods (one for theory, the other for operators) that is not possible at the moment.
Most likely the best way would be to lift ev_op_max_order on theory side (and actually make it part of the ModEv). At that point you won't need any longer.
But ok, let's keep like this for the time being, and do it in a second step. What do you think @felixhekhorn?
There was a problem hiding this comment.
But ok, let's keep like this for the time being, and do it in a second step. What do you think @felixhekhorn?
definitely, this PR is already too big ...
|
fine to merge! |
4 participants
No description provided.