Skip to content

[nfc][hist] clarify bigaus argument order#21901

Merged
guitargeek merged 1 commit intoroot-project:masterfrom
ferdymercury:bigausord
Apr 13, 2026
Merged

[nfc][hist] clarify bigaus argument order#21901
guitargeek merged 1 commit intoroot-project:masterfrom
ferdymercury:bigausord

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

No description provided.

@ferdymercury ferdymercury added the skip ci Skip the full builds on the actions runners label Apr 13, 2026
@ferdymercury ferdymercury changed the title [skip-ci][hist] clarify bigaus argument order [nfc][hist] clarify bigaus argument order Apr 13, 2026
@ferdymercury ferdymercury marked this pull request as ready for review April 13, 2026 08:42
@ferdymercury ferdymercury requested a review from hageboeck as a code owner April 13, 2026 08:42
Copy link
Copy Markdown
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the list in the line just before in the docs not sorted consistently? That's a bit surprising. But well, this new line definitely clarifies things.

@guitargeek guitargeek merged commit a22fe0e into root-project:master Apr 13, 2026
13 checks passed
@ferdymercury ferdymercury deleted the bigausord branch April 13, 2026 20:57
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

Why was the list in the line just before in the docs not sorted consistently? That's a bit surprising.

Not sure what you mean?
bigaus is defined with an order of parameters that differs from ROOT::Math::bigaussian_pdf(...) function
So the docs were right, just missing more detail?

@guitargeek
Copy link
Copy Markdown
Contributor

I mean that I didn't like that the ordering of the parameters passed to the bigaus substitute in TFormula is not consistent with the parameter order of ROOT::Math::bigaussian_pdf, which became obvious from the documentation. We should be more consistent with these things inside ROOT in my opinion.

Of course that's nothing we can change at this point, so it was just a random rant. Sorry 🙂

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

I mean that I didn't like that the ordering of the parameters passed to the bigaus substitute in TFormula is not consistent with the parameter order of ROOT::Math::bigaussian_pdf, which became obvious from the documentation. We should be more consistent with these things inside ROOT in my opinion.

Of course that's nothing we can change at this point, so it was just a random rant. Sorry 🙂

Ahhh yeah, I got you now. Yeah, too late to change.

Maybe the problem was that originally bigaus was an extension of gaus or xygaus so it started from that order
whereas Math::bigaussian was supposed to be centered at zero and later evolved to have also an offset as last parameters.
But @couet might remember the details better.

Btw related: https://root-forum.cern.ch/t/whats-the-predefined-function-bigauss/45572

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip ci Skip the full builds on the actions runners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants