Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add t0 chi2 to comparefitsreport #1613

Merged
merged 2 commits into from
Oct 12, 2022
Merged

Conversation

RoyStegeman
Copy link
Member

As discussed during the meeting today, we should have the t0 chi2 info by default included in the fitreports.

@RoyStegeman RoyStegeman changed the base branch from master to fix_for_overfitmetric October 7, 2022 13:45
@scarlehoff scarlehoff added the run-fit-bot Starts fit bot from a PR. label Oct 10, 2022
@github-actions
Copy link

Greetings from your nice fit 🤖 !
I have good news for you, I just finished my tasks:

Check the report carefully, and please buy me a ☕ , or better, a GPU 😉!

@scarlehoff
Copy link
Member

I'm going to be picky and say that it should be either part of the summary table or completely separated, with its own heading.

@RoyStegeman
Copy link
Member Author

RoyStegeman commented Oct 11, 2022

A single table would of course be the obvious way to format it, but since the t0 settings would be different between entries reportengine cannot take care of it. So either we do this, or we just calculate the different t0 settings inside one of the summary functions out of reach from reportengine (but this seems pretty ugly/bad practice).

But sine you're not alone with this opinion #1613 (comment) I guess I'l place another heading.

Base automatically changed from fix_for_overfitmetric to master October 12, 2022 14:09
@scarrazza scarrazza merged commit cc358ff into master Oct 12, 2022
@scarrazza scarrazza deleted the add_t0chi2_to_comparereport branch October 12, 2022 14:18
@scarlehoff scarlehoff removed the run-fit-bot Starts fit bot from a PR. label Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants