Add support for weighted fraction accumulator #385
Conversation
|
Thanks for this substantial contribution, I will review it next weekend. I think you can replace the initializer to remove the warning. |
| @@ -36,7 +37,8 @@ class fraction { | |||
| using const_reference = const value_type&; | |||
| using real_type = typename std::conditional<std::is_floating_point<value_type>::value, | |||
| value_type, double>::type; | |||
| using interval_type = typename utility::wilson_interval<real_type>::interval_type; | |||
| using score_type = typename utility::wilson_interval<real_type>; | |||
There was a problem hiding this comment.
score_type should not appear here, because it is not a type that needs to be publicly visible.
There was a problem hiding this comment.
This issue should be addressed now. Please take a look.
|
The failing check is due to the job never starting and eventually timing out. |
|
This sometimes happens. I restarted the job. If it continues to fail, we can ignore it. Checking compatibility with an ancient compiler becomes less and less useful as time progresses. |
|
Please let me know if there are any changes I can make to this. |
|
I apologize for being slow with responding. I appreciate your enthusiasm to contribute to Boost.Histogram. I need some time to carefully think about the APIs, because in Boost we have very high standards in regards to stability. It would be a good idea to start a new subfolder called |
|
I moved the |
This PR extends
fractionto weighted samples to address issue #367.I took the approach of creating a new
weighted_fractionclass. This class is a composition of two classes:fractionand a new internal classsum_of_weights_squared. I explain the thought process behind these choices.The classes$\sum (w^2)$ . So, I turned this piece of information into a class
sum,mean, andfractionare non-weighted. Each of these classes could almost function as a weighted class, but requires one additional piece of information: the sum of the weights squared, that is to saysum_of_weights_squared. I created the classweighted_fractionby puttingfractionandsum_of_weights_squaredtogether.The code passes all unit tests (locally), but has
Wpedanticwarnings becausewilson_solveuses a designated initializer (i.e.,wilson_solve({.n_eff=n_eff, .p_hat=p_hat, .correction=correction})). There's likely a good way to avoid getting this warning, while also enforcing input meaning, but I'm missing it.