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

9 mixture distributions #77

Merged
merged 12 commits into from
Oct 31, 2024
Merged

9 mixture distributions #77

merged 12 commits into from
Oct 31, 2024

Conversation

craig-parylo
Copy link
Contributor

Closes #9

First draft to include @anyaferguson 's mixture distributions as an accompanyment to the point-range plots.

image

Settings
image

'Show ECDF' - unchecked (default) shows the probability density, checked shows the cumulative distribution.
image

'Show NEE range' - checked (default) enables a crossbar geom that overlays the results of the national elicitation exercise (similar to the point-range plots)
image

The process to generate mixture distributions takes a couple of seconds to calculate and a few more to render as plots. Not a lot of time in the scheme of things, but something I think would be noticeable to the user. In this PR I've arranged for the mixture distributions to be pre-rendered on the app start-up based on all available schemes to speed up the chart generation.

Note, for the future we may want to add in an invalidation timer to ensure the mixture distributions are regenerated on a routine basis - preferably in sync with the updates to the underlying data.

Known issues

🐛 Mixture distributions are fixed to include all schemes
Pre-calculated mixture distributions (see above) designed to improve UX by increasing responsiveness also means they aren't dynamic, so include the focal scheme and do not respect the comparator schemes selection.

🐛 ggplot2 warning
Displaying the NEE reference range results in the below warning message printed to the console:
Warning in ggplot2::geom_crossbar(data = stats::na.omit(focal_pointrange), : Ignoring unknown aesthetics: width

This is caused by referencing width when setting the aesthetics for the geom_crossbar() - which is my attempt at making the width (actually the height) of the crossbar dynamic to the maximum height of mixture distribution plot. Attempts to resolve include a) removing the width argument and b) setting a fixed width. Results show a) results in a very narrow crossbar to appears as a single line and mid-point is not visible, b) uneven widths relative to the mixture distribution plot across facets - the y-axis is variable scale depending on the maximum density of the distribution plot, so some facets with a more even distribution show a crossbar with large width and others with large peaks have a narrow crossbar width.

@craig-parylo craig-parylo self-assigned this Oct 28, 2024
@craig-parylo craig-parylo added this to the v0.4.0 milestone Oct 28, 2024
@craig-parylo craig-parylo removed this from the v0.4.0 milestone Oct 28, 2024
Copy link
Contributor

@matt-dray matt-dray left a comment

Choose a reason for hiding this comment

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

Cracking, many thanks. We talked about this earlier today. As mentioned, I'm approving it and we can iterate later on the issues you mentioned.

However, can you add one thing before you merge? Could you suppress the 'width' warning and add a comment in the code to say that the warning is known about?

I'll add a few techdebt issues to the repo in light of this:

I've created a more general issue (#80) about how best to describe plots as well, which we also chatted about.

Cheers.

@craig-parylo craig-parylo merged commit 067593c into main Oct 31, 2024
@craig-parylo craig-parylo deleted the 9_mixture_distributions branch October 31, 2024 10:20
@matt-dray matt-dray mentioned this pull request Nov 6, 2024
@matt-dray matt-dray added must MoSCoW priority and removed should MoSCoW priority labels Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request must MoSCoW priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toggle mixture distributions in plots
2 participants