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 counterfactual minus guided/unguided diff at location level #726

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Zapiano
Copy link
Collaborator

@Zapiano Zapiano commented Mar 22, 2024

Followed an approach similar to #452 (comment) but for a spatial level.

Note: Depends on #847

Example of code

using ADRIA
using WGLMakie, GeoMakie, GraphMakie

Makie.inline!(false)

# Load domain
dom = ADRIA.load_domain(...)

# Create scenarios
num_scens = 2^10
scens = ADRIA.sample(dom, num_scens)

# Run model / Load ResultSet
rs = ADRIA.run_scenarios(dom, scens, "45")

# Extract cf_difference_loc for guided and unguided (in that order)
rc = ADRIA.metrics.relative_cover(rs)

# Compute difference between guided and counterfactual using the 0.6-th quantile
gd_res = metrics.ensemble_loc_difference(rc, scens; agg_metric=0.6)

# Compute difference between unguided and counterfactual using the median
ug_res = metrics.cf_difference_loc(rc, scens; diff_target=:unguided)

# Plot maps of difference to the counterfactual
ADRIA.viz.diff_map(rs, gd_res[2, :])
ADRIA.viz.diff_map(rs, ug_res[2, :])

Plots

  • 1024 scenarios
Guided - Counterfactual (0.6-th quantile) Unguided - Counterfactual (median)
gd_cf_diff_map ug_cf_diff_map

@ConnectedSystems
Copy link
Collaborator

Bump @Zapiano

@ConnectedSystems
Copy link
Collaborator

How close are we to merging this?

@Zapiano
Copy link
Collaborator Author

Zapiano commented Sep 13, 2024

How close are we to merging this?

I'll start looking into this now and let you know what's missing or of it's ready soon.

@Zapiano Zapiano marked this pull request as ready for review September 13, 2024 10:11
@Zapiano
Copy link
Collaborator Author

Zapiano commented Sep 13, 2024

@ConnectedSystems This one is ready for another review. Note that now it depends on #847

@Zapiano Zapiano force-pushed the loc-diff branch 3 times, most recently from da1db9d to 9ef5c30 Compare September 20, 2024 05:56
@ConnectedSystems
Copy link
Collaborator

ConnectedSystems commented Sep 21, 2024

Thanks.

An example usage would be appreciated too @Zapiano

@Zapiano
Copy link
Collaborator Author

Zapiano commented Sep 23, 2024

Thanks.

An example usage would be appreciated too @Zapiano

I've added an example in this PR. Once this PR is approved I can add this plot to the documentation on a separate PR.

@Zapiano Zapiano force-pushed the loc-diff branch 2 times, most recently from f1f7e38 to eadec98 Compare October 3, 2024 07:09
@ConnectedSystems
Copy link
Collaborator

I'm not sure the indicated use it the best approach.

It forces only one set of bootstrapped results (only the median?), and forces additional computations to be done (e.g., what if I only want gd_res?)

# Extract cf_difference_loc for guided and unguided (in that order)
rc = ADRIA.metrics.relative_cover(rs)
gd_res, ug_res = ADRIA.metrics.cf_difference_loc(rc, scens)  # <- this method name feels awkward

# Plot cf_difference_loc for unguided and guided
gd_fig = ADRIA.viz.diff_map(rs, gd_res[2, :])  # <- why `[2, :]`? (I know why, but feels awkward)
ug_fig = ADRIA.viz.diff_map(rs, ug_res[2, :])

Instead, I suggest something like the below which, to me, is a cleaner UX.

rc = ADRIA.metrics.relative_cover(rs)

# outcome, scenario spec, percentile value of interest
gd_res = ADRIA.metrics.ensemble_difference(rc, scens, 0.5; timestep=2, scenario_type=:guided)

# outcome, scenario spec, statistical function of interest
ug_res = ADRIA.metrics.ensemble_difference(rc, scens, mean; timestep=2, scenario_type=:unguided)

On the plotting, it kind of feels like we're getting to the point where we want our own YAXArray subtype.

ADRIA.viz.map(rs, gd_res::SomeCustomYAXArray?)  #<- uses different defaults to the "usual" `viz.map()`

@Zapiano
Copy link
Collaborator Author

Zapiano commented Oct 22, 2024

@ConnectedSystems this was updated to address your comments. The description was updated with new example of usage and plots.

@Zapiano
Copy link
Collaborator Author

Zapiano commented Oct 22, 2024

On the plotting, it kind of feels like we're getting to the point where we want our own YAXArray subtype.

ADRIA.viz.map(rs, gd_res::SomeCustomYAXArray?)  #<- uses different defaults to the "usual" `viz.map()`

I'm not sure I understand the purpose of having our own YAXArray subtype.

@ConnectedSystems
Copy link
Collaborator

ConnectedSystems commented Oct 22, 2024

I'm not sure I understand the purpose of having our own YAXArray subtype.

To be able to define our own custom styling for specific use cases and be able to make different styles (e.g., different set of default options) when plotting some arbitrary results (e.g., some_plot(a_matrix)) compared to some_plot(a_general_yaxarray) and some_plot(a_yaxarray_with_context_specific_metadata)

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