Update viz.clustered_scenarios to allow confidence interval plot #482

Zapiano · 2023-09-14T07:18:10Z

Updated functions _clusters_colors and _cluster_color to _get_colors and _get_alphas
Added return type to all functions in this file
Adapted the whole file to the style guide

There is an option to fill or not the area inside the confidence interval - the best visualization option may depend on the situation. The default value of :fill_confint is currently set to true:

This is how it looks with :fill-confint set to false:

Closes #464

ConnectedSystems · 2023-09-14T08:19:36Z

This is how it looks with :fill-confint set to false:

I'm struggling to see where this is useful - I suggest :aggregate or :summarize (defaulting to true) which plots either the individual series or the aggregated version with filled confidence intervals.

Incidentally, I'm not against confint, but also putting ci (or CI) as an option as it's an established acronym.

ConnectedSystems

Thanks Pedro, I think some minor tweaks (mostly to get rid of push!()) are needed.

ext/AvizExt/viz/clustering.jl

- Updated functions _clusters_colors and _cluster_color to _get_colors and _get_alphas - Added return type to all functions in this file - Adapted the whole file to the style guide

Zapiano · 2023-09-17T23:21:36Z

This is how it looks with :fill-confint set to false:

I'm struggling to see where this is useful - I suggest :aggregate or :summarize (defaulting to true) which plots either the individual series or the aggregated version with filled confidence intervals.

100% agree.

Incidentally, I'm not against confint, but also putting ci (or CI) as an option as it's an established acronym.

I know about ci, but I really think it's better to avoid acronyms when possible.

Also remove option to not fill confint when it is plotted

Zapiano · 2023-09-18T02:25:37Z

@ConnectedSystems ready for another review.

About the timestep() function:
This function is already being used (line 39) when the Axis is created, BUT I have to pass an array to band! with x-axis values. You can see that the previous plots doesn't have x-axis labels. I had to use this range (line 84) to fix that. This is the final result:

:summarize = true	:summarize = false

ConnectedSystems

Some things to think about, but overall looks good!

ext/AvizExt/viz/clustering.jl

ConnectedSystems · 2023-09-18T08:23:11Z

ext/AvizExt/viz/clustering.jl

+    for (i, cluster) in enumerate(unique(clusters))
+        n_scens = count(clusters .== cluster)
+        base_alpha = 1.0 / (n_scens * 0.05)
+        alphas[i] = max(min(base_alpha, 0.6), 0.1)
+    end


Thinking about this now, shouldn't the weighting be based on total number of scenarios?
What we have right now could lead to a smaller cluster hiding a larger cluster (unless I'm misinterpreting the code here?)

I don't think a small cluster would hide a large one because alpha is, at the most, 0.6 (for small clusters) but in this case the cluster has only a few lines.

Considering the result (figure above) it seems to me that this works fine.

That said, if I would do this now, I would probably try something like:

base_alpha = 1 - count(clusters .== cluster)/length(clusters) alphas[i] = max(min(base_alpha, 0.6), 0.1)

But I think this change is out of the scope of this PR so if you think we should change this I suggest we open an issue, what do you think?

No need for an issue if you're going to submit a PR directly after (but please do if your not planning to make changes immediately).

By hiding, take a look at the example figures provided here. See how the orange trajectories are blocked from view.

#492 (comment)

Maybe we plot smaller clusters last?

You wrote "smaller cluster hiding a larger cluster" but in this figure what I see is a larger cluster hiding a smaller cluster, right? But yes, I think a good improvement would be to plot clusters in order from larger to smaller.

Let me open a different PR to do that and also try to change the way this alpha is computed and test some different cases.

You wrote "smaller cluster hiding a larger cluster" but in this figure what I see is a larger cluster hiding a smaller cluster, right?

Yes, correct, accidentally swapped those, sorry for the miscommunication

Maybe the problem with the figures you sent from PR 492 wasn't the alpha weight.

If you see my last commit - and the commit message - you will see what happened:

Currently we are iterating over the cluster with a for cluster in unique(clusters) but if clusters vector is something like [2, 1, 1, 3, 2], then unique(clusters) will return [2, 1, 3], and then cluster number 2, for instance, will get the weight that should be assigned to cluster 1, because we are using the index of the cluster inside the vector as it was the number of the cluster itself.

Update: I just tested here and that doesn't solve the visualization problem. I think plotting larger clusters before smaller ones will solve that though.

Previously we were using the index of unique(clusters) array to find clusters colors, plot clusters and render the legend. But depending on the order they appear in clusters vector this could led to mistakes, like cluster number 2 being the first in this vector (meaning it has index 1). That was fixed in a previous commit but the legend wasn't.

Zapiano · 2023-09-20T03:58:11Z

@ConnectedSystems

I just made a small change, so this summary visualization works with other types of timeseries, not only scenarios. This is how it looks for coral cover and different sites (target cluster == 1 and non-target == 2):

Coral Cover (target high values)	Shelter Volume (target low values)

Update version number to 0.9.0

Add .JuliaFormatter.toml to .gitignore

- Updated functions _clusters_colors and _cluster_color to _get_colors and _get_alphas - Added return type to all functions in this file - Adapted the whole file to the style guide

Also remove option to not fill confint when it is plotted

Previously we were using the index of unique(clusters) array to find clusters colors, plot clusters and render the legend. But depending on the order they appear in clusters vector this could led to mistakes, like cluster number 2 being the first in this vector (meaning it has index 1). That was fixed in a previous commit but the legend wasn't.

…r-viz

ConnectedSystems · 2023-09-22T07:40:21Z

Hi @Zapiano , you don't need to merge in unrelated changes if they aren't relevant to the PR.

Zapiano · 2023-09-22T07:51:30Z

Hi @Zapiano , you don't need to merge in unrelated changes if they aren't relevant to the PR.

I know... it was by accident

Zapiano added enhancement New feature or request maintenance labels Sep 14, 2023

Zapiano requested a review from ConnectedSystems September 14, 2023 07:18

Zapiano self-assigned this Sep 14, 2023

ConnectedSystems requested changes Sep 14, 2023

View reviewed changes

ext/AvizExt/viz/clustering.jl Show resolved Hide resolved

ext/AvizExt/viz/clustering.jl Outdated Show resolved Hide resolved

ext/AvizExt/viz/clustering.jl Outdated Show resolved Hide resolved

ext/AvizExt/viz/clustering.jl Outdated Show resolved Hide resolved

Update viz.clustered_scenarios to allow confidence interval plot

e7eef96

- Updated functions _clusters_colors and _cluster_color to _get_colors and _get_alphas - Added return type to all functions in this file - Adapted the whole file to the style guide

Zapiano force-pushed the agg-cluster-viz branch from 7732f85 to e7eef96 Compare September 17, 2023 23:12

Zapiano added 7 commits September 18, 2023 09:42

Change name of option to plot confint for clusters to summarize

68bc155

Also remove option to not fill confint when it is plotted

Add double space between imports and code

d3ff39c

Stop using push! to fill leg_entry vector

4855b72

Rename xs to x_timestamp

3efd859

Change data type to NamedDimsArray

651926d

Fix x-axis not being displayed for _plot_clusters_confint!

be959a2

Change x_timesteps type to UnitRange

12f6f38

ConnectedSystems requested changes Sep 18, 2023

View reviewed changes

Zapiano added 3 commits September 19, 2023 09:25

Change plot functions kwargs order to be consistent

8c164f4

Refactor to follow styleguide

649db44

Refactor to use timesteps function

89c524b

Zapiano force-pushed the agg-cluster-viz branch from c1c6258 to 89c524b Compare September 18, 2023 23:30

Zapiano added 2 commits September 20, 2023 13:36

Add support for site timeseries

031a6d9

Rosejoycrocker and others added 5 commits September 21, 2023 14:10

Update version number

70dc0c4

Merge pull request #497 from open-AIMS/v-0.9.0

3f1f81d

Update version number to 0.9.0

Add .JuliaFormatter.toml to .gitignore

def9314

Merge pull request #499 from open-AIMS/gitignore-juliaformatter

28e8abf

Add .JuliaFormatter.toml to .gitignore

Update viz.clustered_scenarios to allow confidence interval plot

02b8c6e

- Updated functions _clusters_colors and _cluster_color to _get_colors and _get_alphas - Added return type to all functions in this file - Adapted the whole file to the style guide

Zapiano added 13 commits September 22, 2023 13:35

Change name of option to plot confint for clusters to summarize

c7665b0

Also remove option to not fill confint when it is plotted

Add double space between imports and code

d296de0

Stop using push! to fill leg_entry vector

cf52fcc

Rename xs to x_timestamp

36ed0a4

Change data type to NamedDimsArray

81814ff

Fix x-axis not being displayed for _plot_clusters_confint!

0191d38

Change x_timesteps type to UnitRange

829426e

Change plot functions kwargs order to be consistent

bfff463

Refactor to follow styleguide

3153a01

Refactor to use timesteps function

fc72b75

Add support for site timeseries

23e0005

Merge remote-tracking branch 'origin/agg-cluster-viz' into agg-cluste…

ec8180b

…r-viz

ConnectedSystems approved these changes Sep 22, 2023

View reviewed changes

ConnectedSystems merged commit 8b322bd into main Sep 22, 2023

Zapiano deleted the agg-cluster-viz branch September 25, 2023 00:38

Zapiano mentioned this pull request Dec 6, 2023

Register v0.10.0 #626

Closed

Update viz.clustered_scenarios to allow confidence interval plot #482

Update viz.clustered_scenarios to allow confidence interval plot #482

Uh oh!

Conversation

Zapiano commented Sep 14, 2023

Uh oh!

ConnectedSystems commented Sep 14, 2023

Uh oh!

ConnectedSystems left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Zapiano commented Sep 17, 2023

Uh oh!

Zapiano commented Sep 18, 2023

Uh oh!

ConnectedSystems left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ConnectedSystems Sep 18, 2023

Choose a reason for hiding this comment

Uh oh!

Zapiano Sep 18, 2023

Choose a reason for hiding this comment

Uh oh!

Zapiano Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

ConnectedSystems Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zapiano Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

ConnectedSystems Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

Zapiano Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

Zapiano Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zapiano commented Sep 20, 2023

Uh oh!

ConnectedSystems commented Sep 22, 2023

Uh oh!

Zapiano commented Sep 22, 2023

Uh oh!

Uh oh!

ConnectedSystems Sep 19, 2023 •

edited

Loading

Zapiano Sep 20, 2023 •

edited

Loading