search

flav-io / flavio

A Python package for flavour physics phenomenology in the Standard model and beyond
http://flav-io.github.io/
MIT License
71 stars 62 forks source link

Slow multi-thread calculation of SM covariance for fit with SMEFT WCs #46

Closed peterstangl closed 6 years ago

peterstangl commented 6 years ago

After defining a FastFit instance fit with SMEFT WCs, calling

fit.get_sm_covariance()

is considerably faster than the parallelized version

fit.get_sm_covariance(threads=4)

This seems to be due to the fact that the single thread version calls self.get_predictions_array with arguments par=False, nuisance=True, wc=False, https://github.com/flav-io/flavio/blob/9a6b071d9f88362dffdbdfd5faec9873da9ef118/flavio/statistics/fits.py#L539 while the multi thread version calls self.get_predictions_array without arguments, thus using the default values par=True, nuisance=True, wc=True, https://github.com/flav-io/flavio/blob/9a6b071d9f88362dffdbdfd5faec9873da9ef118/flavio/statistics/fits.py#L544 https://github.com/flav-io/flavio/blob/9a6b071d9f88362dffdbdfd5faec9873da9ef118/flavio/statistics/fits.py#L319 where the crucial argument is wc=True. Due to this argument, running in SMEFT is performed for each calculation of observable predictions, which can increase the calculation time by several orders of magnitude.

peterstangl commented 6 years ago

This is related to issue #42, but it is a somewhat different problem.

I'm not sure how the bug that a wc_function is assumed to return vanishing Wilson coefficients for vanishing arguments is exactly related to this.

DavidMStraub commented 6 years ago

Hm, this is a bit surprising to me since even when using wc=True, the Wilson coefficients should still have value 0, so no running should be performed. It could be that it is just the part of the SMEFT running that determines the SM parameters that slows it down.

How to tried snakeviz to check which function calls (in wilson) take most of the time?

peterstangl commented 6 years ago

The Wilson coefficients have value 0. However, if wc=True, the wc_obj that is subsequently used to calculate observable predictions is defined by https://github.com/flav-io/flavio/blob/0208eb093b785b400ee1d54d21a1a7a6d4067a9c/flavio/statistics/fits.py#L328-L329 while for wc=False it is given by https://github.com/flav-io/flavio/blob/0208eb093b785b400ee1d54d21a1a7a6d4067a9c/flavio/statistics/fits.py#L330-L331 The former is a flavio.physics.eft.WilsonCoefficients instance with initial values containing eft, basis, and scale set according to the definition of the FastFit instance. It contains all Wilson coefficients included in the fit with their values set to 0. The latter is also a flavio.physics.eft.WilsonCoefficients instance but with no initial values set: https://github.com/flav-io/flavio/blob/0208eb093b785b400ee1d54d21a1a7a6d4067a9c/flavio/physics/eft.py#L215

Those two different instances of flavio.physics.eft.WilsonCoefficients behave differently when their method get_wc is called (the same applies to their get_wcxf method introduced in https://github.com/DavidMStraub/flavio-dev/commit/7cbccba543a187cc008501e58d25f448aa258bbf). In particular, for the _wc_sm-instance, self.wc is None, such that no running is performed, while for the get_wc_obj(x)-instance, self.wc is not None and running is performed even though all Wilson coefficients have value 0: https://github.com/flav-io/flavio/blob/0208eb093b785b400ee1d54d21a1a7a6d4067a9c/flavio/physics/eft.py#L206-L208

DavidMStraub commented 6 years ago

OK you are right! Prior to v0.28 (before wilson), zero WCs were not run so this made no difference. So this should actually be implemented in wilson. But in addition it certainly makes sense to fix this in flavio as well by using wc=False.

peterstangl commented 6 years ago

The bug should be solved in PR #47.

In addition, it might be useful to change https://github.com/flav-io/flavio/blob/0208eb093b785b400ee1d54d21a1a7a6d4067a9c/flavio/physics/eft.py#L206 into

if not self.wc or not any(self.wc.values.values()):

However, due to the changes introduced in https://github.com/DavidMStraub/flavio-dev/commit/7cbccba543a187cc008501e58d25f448aa258bbf, it might be more reasonable to apply this modification only after the new get_wcxf method has been added in the flavio master branch.

DavidMStraub commented 6 years ago

Oh right, it's time to cherry-pick that commit.

DavidMStraub commented 6 years ago

Thinking twice about it, I added your suggested change without cherry-pciking the other one.

DavidMStraub commented 6 years ago

Closed by #47.