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 "triangle" plot #831

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Add "triangle" plot #831

merged 2 commits into from
Jun 4, 2024

Conversation

Sbozzolo
Copy link
Member

@Sbozzolo Sbozzolo commented Jun 1, 2024

Let's make it square. :)

In this PR, I add a plot that compares our model with the best model (where best is the one that minimizes RMSEs). Errors are reported in terms of median RMSE

image

@Sbozzolo Sbozzolo requested a review from szy21 June 1, 2024 00:03
@Sbozzolo Sbozzolo force-pushed the gb/leaderboard branch 4 times, most recently from 665b4ea to cfd2dba Compare June 1, 2024 00:10
Copy link
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks! Maybe we can get rid of the best model to make the plot less busy? Would it be hard to do the actual triangle plot (we don't need to do it now)?

@Sbozzolo
Copy link
Member Author

Sbozzolo commented Jun 3, 2024 via email

@szy21
Copy link
Member

szy21 commented Jun 3, 2024

My understanding of the spirit of the triangle plot is to visually compare various models, so I thought that having the best model was important. As for the triangles, what I can easily do is make them squares. That is, condense all the squares I have now as sub-squares of one square (instead of sub triangles of one square). Making triangles would add more more code.

On Mon, Jun 3, 2024, 6:55 AM Zhaoyi Shen @.> wrote: @.* approved this pull request. Looks good to me, thanks! Maybe we can get rid of the best model to make the plot less busy? Would it be hard to do the actual triangle plot (we don't need to do it now)? — Reply to this email directly, view it on GitHub <#831 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACF6E7OTZCNUDEJNNC2FOC3ZFRYW5AVCNFSM6AAAAABITTV4S6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJTHE2TENZXGU . You are receiving this because you authored the thread.Message ID: @.***>

Sounds good, let's keep the best model. And let's move to sub-squares once we have more variables (unless it's very easy to do).

@Sbozzolo Sbozzolo enabled auto-merge June 4, 2024 01:47
@Sbozzolo Sbozzolo force-pushed the gb/leaderboard branch 4 times, most recently from 4050f08 to 963f1bc Compare June 4, 2024 21:03
Fixes compatibility with ClimaCore
@Sbozzolo Sbozzolo merged commit eb51409 into main Jun 4, 2024
6 checks passed
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.

2 participants