Adding Double Crystal Ball function

[andreas01060]

Hi, I need to fit a double crystal ball for a project. Hence, this pull request. I think I have added all necessary
lines of codes to accommodate this.
Thank you

[eduardo-rodrigues]

Hi @andreas01060 and @HDembinski, I just bumped into this old PR. Any hope to get it sorted and merged soon? It's a nice addition.

[andreas01060]

Hi @eduardo-rodrigues, I couldn't get it past the tests, so in the end I just cloned my probfit version and added it to my python packages (and it works). I didn't really understand why the tests failed, that's why I abandoned on the PR. When I have some time, I will try again though.

[henryiii]

Tests fail because ProbFit does not work with iMinuit > 1.2, I believe. I think we probably should pin the iMinuit version, then try to fix this (and then unpin the version).

[eduardo-rodrigues]

Thanks for your reply @andreas01060. Seems @henryiii has found the source of the problem and PR #89 aims at sorting it out. Fingers crossed ...

[eduardo-rodrigues]

Hi @marinang, you said on the PyHEP Gitter channel that "I am using probfit, also with python 3.7". Maybe you have useful input to this discussion?

[marinang]

Hi @eduardo-rodrigues. I have a fork of profit and I modified some stuffs, like in #84, and some additions for future PR (johnsonSU pdf, add constraints to likelihood), but nothing crucial or I don't recall. As you said I have python 3.7 and iminuit 1.3.2.

[eduardo-rodrigues]

So you say that with #84 you actually got the builds OK? If that's the case it would make sense to have a PR with that, to try and fix Travis ...

Cc @henryiii for info.

[eduardo-rodrigues]

Hi @andreas01060, can you rebase this PR to resolve the conflicts, now that #91
went in?

[andreas01060]

Hi @eduardo-rodrigues , I resolved the conflicts, but the check is still failing..

[marinang]

The plotting test are failing, because you replaced all the plotting test with gaussian with double crystal ball right, in test_plotting.py? I think you let those tests as they were with the gaussian.

[andreas01060]

Hi @marinang, yes true , but now it's back to how how it first was: I deleted those changes and put back in the original ones.

[marinang]

@andreas01060 did you already make the changes? I don’t see a new commit.

[eduardo-rodrigues]

That's right, you seem to have replaced tests whereas you should be adding new tests for the double CB. I believe the replacement of tests was in since the first commit.

[andreas01060]

Hi sorry for the delay, Is it okay now? I was confused, I thought the various plotting tests should also be applied on any new functions implemented..

[henryiii]

Whoever merges this should make sure it's a squash and merge...

[henryiii]

Looks pretty good to me. Adds a bit of unneeded whitespace, but otherwise seems to be nicely done.

[henryiii]

Just curious though, can't you just add two crystal ball functions? I believe this even happens at the compiled code level due to Cython's cpdef.

>>> dg = probfit.AddPdf(probfit.crystalball, probfit.crystalball, prefix=['f_','g_'], skip_prefix=['mean', 'sigma'])
>>> probfit.describe(dg)
['x', 'f_alpha', 'f_n', 'mean', 'sigma', 'g_alpha', 'g_n']
>>> dg(10, 1, 2, 10, 2, 1, 1) / 2 
1.0
>>> dg(11, 1, 2, 10, 2, 1, 1) / 2
0.8824969025845955

I'd rather have a small set of core elements that can be composed, I think.

[andreas01060]

I think this might be just the case from the particular example you give. If you take x such that it's in the tails, I don't think it will work. Also when the alpha and n differ for the two tails, this complicates it more.

[henryiii]

With all the examples you've provided, I get identical results to floating point precision. Can you add an example with different values in the two tails to the test?

[eduardo-rodrigues]

Henry's suggestion for extra tests is a good one. On the doc side I would suggest that you add this is a double CB of common mean and core resolution (OK, obvious from the signature, but still good to add for clarity).

Otherwise this seems ready to go. Nice.

[eduardo-rodrigues]

Thumbs up from me.

[eduardo-rodrigues]

Just pinging you @HDembinski, for when you have the time, as this seems ready to go.

[HDembinski]

@andreas01060 Thank you for contributing this!

[andreas01060]

Thank you all for helping me out! (This was my first time using github, so sometimes it was a bit cryptic for me. )