wilson-eft / wilson

A Python package for the running and matching of Wilson coefficients above and below the electroweak scale
https://wilson-eft.github.io
MIT License
26 stars 19 forks source link

Turn `smeftutil` into instance of new class `EFTutil` and add new `wetutil` instance #92

Closed peterstangl closed 2 years ago

peterstangl commented 2 years ago

This PR refactors code that is so far used only in the context of solving the SMEFT beta functions in order to make it suitable for solving the WET beta functions.

peterstangl commented 2 years ago

The symmetry class with key 9 in smeftutil.py (the dim-5 Weinberg operator) was different from class 9 in wetutil.py (B violating 4f operator antisymmetric in the first two indices). In commit https://github.com/wilson-eft/wilson/pull/92/commits/ce7f88cd7e9ae429e96fb6352fa681d69fe14442, the latter is renamed to class 71 since it is similar to class 7 (B violating 4f operator symmetric in the first two indices).

peterstangl commented 2 years ago

Why was there no symmetrization function for the class 41 before?

The 41 class does not appear in SMEFT Warsaw basis and for WET the full symmetrization is not implemented so far.

This PR is actually still work in progress. So I converted it to a draft.

peterstangl commented 2 years ago

This PR is now ready to be reviewed. The only things that are still missing are unit tests for the newly added features. Let me explain what this PR is doing:

In the new EFTutil class, the functions from the old smeftutil have been generalized as follows:

Apart from these modifications, there are some things that could be changed later but are kept like they are for the moment:

@dvandyk @jasonaebischerGIT and maybe also @DavidMStraub please take a look. I would like to merge this soon. Based on these changes, I am planning to add a new feature that automatically extracts the WET ADMs from the WET beta functions in order to have an independent extraction that should finally fix issue #54 and make sure that no similar issues exist.

DavidMStraub commented 2 years ago

Will take a look.

DavidMStraub commented 2 years ago

In general, looks really good.

The extraction of symmetry classes from the WCxf coefficient list is quite impressive. However it worries me a bit that now nowhere in the code, nor in the EFT definition, can the "correct" symmetry classes be seen; having the symmetry classes hard-coded allows in principle for a powerful cross-check of the correctness of the non-redundant WCxf basis. Of course you can argue that the Warsaw/JMS bases have been checked to be correct once and for all, are known to be correct, and will never change. But then it's a bit hard to see why you would do this complicated extraction (which I suspect slows down module import quite a bit) every time rather than just hard-coding the known result. Then again, it's nice that you can in principle use this for arbitrary EFTs (right?).

Purely stylistic comment, since common is a new module and screwing up diffs is not a concern, you might want to consider it formatting it with black, might make some things more readable.

peterstangl commented 2 years ago

Thank you @DavidMStraub for going through the PR!

The extraction of symmetry classes from the WCxf coefficient list is quite impressive. However it worries me a bit that now nowhere in the code, nor in the EFT definition, can the "correct" symmetry classes be seen; having the symmetry classes hard-coded allows in principle for a powerful cross-check of the correctness of the non-redundant WCxf basis.

At the moment such a cross-check is not done as far as I know. This means that there was in principle no guarantee that the previously hard-coded symmetry classes agree with those of the non-redundant WCxf basis. One reason for the automatic extraction was that now everything is completely based on WCxf and no separate (and possibly different) definitions are used anymore. That being said, I agree that a cross-check of the correctness of the non-redundant WCxf basis would make even more sense now. A simple solution could be to hard-code the correct symmetry classes inside a unit test that would compare this to the automatically extracted symmetry classes. This would then not only test the correctness of the WCxf implementation but also test the function that does the automatic extraction.

But then it's a bit hard to see why you would do this complicated extraction (which I suspect slows down module import quite a bit) every time rather than just hard-coding the known result.

The extraction might look a bit complicated, but on my laptop it only takes 1.3 ms for WET and 0.8 ms for SMEFT. So it would slow down the import of the module only by about 2 ms, which I think is acceptable :wink: I think having redundant data in a unit-test that uses this data as a cross-check might be better than basing the implementation on redundant data (which would be already available from the wcxf basis definition).

Then again, it's nice that you can in principle use this for arbitrary EFTs (right?).

Yes, exactly. It could be used for any EFT defined in WCxf.

Purely stylistic comment, since common is a new module and screwing up diffs is not a concern, you might want to consider it formatting it with black, might make some things more readable.

Good point! I'll run black on the new common module.

DavidMStraub commented 2 years ago

So it would slow down the import of the module only by about 2 ms, which I think is acceptable

Agreed!

peterstangl commented 2 years ago

I noticed that arrays2wcxf_nonred actually returned Wilson coefficients that are not in the non-redundant basis. So far, these unwanted Wilson coefficients in the output of arrays2wcxf_nonred were then remove afterwards at several places in the code. I think this behavior of arrays2wcxf_nonred is unexpected and can be confusing. I have changed it in https://github.com/wilson-eft/wilson/pull/92/commits/5e544515f0778d8099c4f751a991e935c35af830 such that only non-zero Wilson coefficients from the non-redundant basis are returned. This then also simplifies the code at several other places. @DavidMStraub do you know if there was actually a reason for this kind of behavior? I didn't find any place in the code where this seemed to be necessary. The only place where the unwanted Wilson coefficients were actually not removed is in smeft_evolve_continuous. But I think we also do not want these Wilson coefficients in the output there.

DavidMStraub commented 2 years ago

No, I don't remember (which doesn't mean a lot) but also don't see a reason in the code.

peterstangl commented 2 years ago

OK, thanks for the confirmation @DavidMStraub!

I have also added new tests (including the hard-coded symmetry classes for both SMEFT and WET) and I think this PR is ready to be merged.