Add functions to spatially visualise individual and aggregate criteria for location selection #650
Conversation
|
@Rosejoycrocker when I save the file |
Hi @Zapiano, I recently changed laptops and had to set up my formatter again. I thought I finally had it working but obviously not, I'll look into it again. |
| # Arguments | ||
| - `g` : Figure GridPosition or GridLayout | ||
| - `rs` : Result set | ||
| - `S` : A normalised decision matrix calculated using decison.decision_matrices |
There was a problem hiding this comment.
Is this name S being used in other places as well? Because it is a very opaque name. Why not call it decision_matrix?
There was a problem hiding this comment.
SE and SH are used for the seeding and shading matrices respectively, but I'll change it to decision_matrix for clarity
| axis_opts::Dict = Dict(), | ||
| ) | ||
| if length(rs.site_data.site_id) != size(S, 1) | ||
| error("Only unfiltered decision matrices can be plotted.") |
There was a problem hiding this comment.
Maybe it's just because I don't have context, but what do you mean by "unfiltered decision matrices" ?
There was a problem hiding this comment.
The decision matrices can be filtered based on thresholds for different criteria (like heat stress and wave stress) but this means they have less sites than the whole site set, which means when you go to plot them the geodata (created from the result set) doesn't match the number of sites when plotting using map. Alternatively, filtered sites could be set as 0.0 or something, but they'd have to be added back into the decision matrix because these are automatically filtered in create_decision_matrix.
| opts[:colorbar_limits] = get(opts, :colorbar_limits, (0.0, 1.0)) | ||
|
|
||
| n_criteria::Int64 = length(criteria) | ||
| n_rows = n_cols = ceil(Int64, sqrt(n_criteria + 1)) |
There was a problem hiding this comment.
I think you should consider using _calc_gridsize instead. Not only it is the standard BUT is has a max_cols param to prevent each map from being too small in case we have too many criteria. Since you're plotting all criteria plus one aggregate, you can call _calc_gridsize(n_criteria + 1) and you should be fine. (:
| s_col = 1 | ||
|
|
||
| for row in 1:n_rows, col in 1:n_cols | ||
| if step > n_criteria |
There was a problem hiding this comment.
If you use _calc_gridsize you wont need this if anymore.
| axis_opts = axis_opts_temp | ||
| ) | ||
|
|
||
| step += 1 |
There was a problem hiding this comment.
If you use _calc_gridsize you won't need this step anymore.
src/analysis/decision.jl
Outdated
| rs.dhw_stats[RCP](; stat = "mean"), (:dhw_scenario) | ||
| ) | ||
|
|
||
| TP_data = rs.connectivity_data[RCP] # connectivity matrix |
There was a problem hiding this comment.
if this is the connectivity matrix, what does TP stand for?
src/analysis/decision.jl
Outdated
| rs.dhw_stats[RCP](; stat = "mean"), (:dhw_scenario) | ||
| ) | ||
|
|
||
| TP_data = rs.connectivity_data[RCP] # connectivity matrix |
There was a problem hiding this comment.
If I understand correctly, you're only using this TP_data multiplied by site_k_area, so you could do that here instead and not when calling the function. It would increase readability.
There was a problem hiding this comment.
TP_base is what the connectivity matrix is called in the connectivity calcs, the TP stands for transfer probability :
ADRIA.jl/src/ecosystem/connectivity.jl
Lines 171 to 176 in 2830840
but you're right it's not a very transparent name. I'll change it to
conn_data which is also used in other places in ADRIA.
src/analysis/decision.jl
Outdated
|
|
||
| TP_data = rs.connectivity_data[RCP] # connectivity matrix | ||
| connectivity_data = connectivity_strength( | ||
| TP_data .* site_k_area(rs), vec(loc_coral_cover), TP_data |
There was a problem hiding this comment.
isn't loc_coral_cover a vector already? why do you need this vec here?
There was a problem hiding this comment.
Good point, not sure why this is here
src/analysis/decision.jl
Outdated
|
|
||
| # Criteria for strongest larval sources to priority locations | ||
| priority_source_criteria = ADRIA.decision.priority_predecessor_criteria( | ||
| strong_pred, vec(rs.sim_constants["priority_sites"]), length(site_ids) |
There was a problem hiding this comment.
since you are using vec(rs.sim_constants["priority_sites"]) in more than one place, you could extract it to a variable.
There was a problem hiding this comment.
It's only used once, the other usages is vec(rs.sim_constants["priority_zones"])
src/ecosystem/connectivity.jl
Outdated
|
|
||
| return (in_conn=C1, out_conn=C2, strongest_predecessor=strong_pred) | ||
| function connectivity_strength( | ||
| TP_base::AbstractMatrix{<:Union{Float32, Float64}} |
There was a problem hiding this comment.
| TP_base::AbstractMatrix{<:Union{Float32, Float64}} | |
| TP_base::AbstractMatrix{<:Real} |
There was a problem hiding this comment.
I recommend keeping the union: allows the compiler to specialise for just two types (Float32 and Float64) compared to having to support all possible Reals
Rosejoycrocker
force-pushed
the
mcda-visualisations
branch
from
c9fca07
to
679415b
Compare
Hi @Zapiano, I think I have the formatter working now (ended up doing a full uninstall and reinstall of VS code), but let me know if some things look odd. I've addressed your comments and also changed some things suggested by @ConnectedSystems, such as making |
Rosejoycrocker
force-pushed
the
mcda-visualisations
branch
3 times, most recently
from
ef520b8
to
c0254be
Compare
Add docs for `store_conn` and add detail to docs for `store_env_summary`.
…culation into separate functions to avoid repetition
…rid of spatial heatmaps Label dimensions with full site id to avoid errors with `viz.map`
…ded figure object Also fix how `axis_opts` is defined to avoid overwriting inputs Add documentation for `decision_matrices`
Change `decision.decision_matrices` to give both intervention matrices as output Add in plotting of scores in `viz.decision_matrices()` and change to square plotting grid
Add title to aggregate score plot
…_sites` in criteria constuctors ( as these are their types in the result set) + formatting
Add decison matrix creation from resultset to analysis as new `decision.jl" file Remove `decision_matrices` from "location_selection.jl"
…quired Due to changes in how locations are deemed as having sufficient space
… fogging This allows ease of extraction for decision matrix labelling Plus, update criteria weights docs Add use of separate ingoing and outgoing connectivity weights for fogging to mcda
…ion matrices for plotting
…at no filtering occurs when plotting seed decision matrix Update example and test scenarios with new parameters and parameter names
This reverts commit 537f498.
…usted So all plots in `viz.decision_matrices` have the same colorbar range. This requires removing `color_range`which was an equivalent variable, previously set to `(0.0,maximum(data))` Add use of colorbar_limits ro `viz.decision_matrices`
Remove `s_row_`s_col` statement as no longer necessary Remove unnecessary catch for the case when one figure is plotted (the least that can be plotted is 2)
+ fix missing variables in doc string
Revert "Remove unnecessary `vec()`" This reverts commit 591daf3. Rename `TP_data` `conn_data` and remove unnecessary `vec()`
This reverts commit 8ecbbcd.
…ction and `mcda_func` for scores calculation Change output to Dict of decision matrices and scores for each intervention
…functions as no longer needed inside decision module
Move `decision_matrices` to `decision` module Move `decision_matrices` into decision module Move around file compilation order to allow this in `ADRIA.jl`
…readable criteria names for labels Plus add criteria names as optional inputs so less than total set of criteria can be plotted
Rosejoycrocker
force-pushed
the
mcda-visualisations
branch
2 times, most recently
from
3c593a6
to
3f3a10f
Compare
Rosejoycrocker
force-pushed
the
mcda-visualisations
branch
from
3f3a10f
to
353032c
Compare
|
Bump! Where did we end up on this @Zapiano ? |
|
@Rosejoycrocker this needs a rebase before I review again. I'm so sorry it took me so long... I strongly suggest you don't simply rebase it though, because
I think it might be more productive and avoid mistakes to just create a Let me know if you have any questions (or if you hate me now) |
@Zapiano honestly I forgot about this because it's been 8 months since I responded to your review and naturally much has changed in Main since then. I agree the best approach is probably to start a new branch although I won't be able to give time to this until October. Of course I don't hate you! |
|
#893 now addresses this |
Closes #620
Adds
viz.decision_matricesto visualise specified criteria and aggregate criteria spatially from a decision matrix, aggregate score vector and result set.Adds
analysis.decision_matriceswhich calculates decision matrices for seeding and fogging from a result set and outputs these asNamedDimsArrayslabelled using the criteria weightings names.Some changes to criteria weights names have also been added for consistency and ease of labelling the decision matrices as
NamedDimsArray. These changes will make addressing #624 easier.The weighting for ingoing and outgoing connectivity for fogging has been split into two separate weightings
fog_in_connectivityandfog_out_connectivityfor consistency with seeding. The weightscoral_cover_lowandcoral_cover_highhave been renamedseed_coral_cover_lowandfog_coral_cover_highfor consistency with other weights names. The weights ordering inCriteriaWeightshas been reordered so thatcriteria_paramsextracts the weights names in the correct order to label the decision matrices.