Remove Copras method due to erroring #359
Conversation
- Copras should be equivalent to Topsis when only maximisation is involved (of criteria) - This is probably what caused it to error (possibly min value is set as 0 because no criteria are minimised and then score is divided by this, giving NaNs).
|
Tests all passing on my end |
| @@ -45,7 +45,7 @@ Base.@kwdef struct Intervention{N,P,P2} <: EcoModel | |||
| # Integer values have a +1 offset to allow for discrete value mapping | |||
| # (see `set()` and `map_to_discrete()` methods) | |||
| # Bounds are defined as floats to maintain type stability | |||
| guided::N = Param(0, ptype="integer", bounds=(-1.0, 21.0 + 1.0), dists="unif", | |||
| guided::N = Param(0, ptype="integer", bounds=(-1.0, length(methods_mcda) + 1.0), dists="unif", | |||
There was a problem hiding this comment.
Using the const number of available methods instead to avoid repeated editing
| JMcDM.MEREC.MERECMethod, | ||
| JMcDM.ELECTRE.ElectreMethod, | ||
| JMcDM.PROMETHEE.PrometheeMethod, | ||
| JMcDM.Topsis.TopsisMethod, |
There was a problem hiding this comment.
Rose, I've removed TOPSIS and VIKOR from consideration, but did you intend to keep these in for a reason I wasn't aware of?
There was a problem hiding this comment.
That probably makes sense to avoid overlap. I kept them in incase we wanted a "fast version" because they are quite a bit faster than the JMcDM implementations, but it's not very noticable when running a lot of simulations.
There was a problem hiding this comment.
it's not very noticable when running a lot of simulations.
That's when it should be very noticeable? I'm noting roughly an extra minute in run times compared to when we were only considering the three methods for 1024 scenarios.
I'll do a more in-depth profiling, but noting that seemingly small increases has a large implication for scenario/sensitivity analysis.
There was a problem hiding this comment.
@Rosejoycrocker some minor comments for your review. If all okay then I will merge.
Confirm tests pass here too.
|
All good on my end :) |
|
FYI, PSIMethod looks promising, at least for coral cover (14th in the methods list). |
|
Oh cool, PSI is based on approximate pareto optimal solutions in the criteria space. |
Closes #357
Copras method was causing issues due to giving all NaNs for the criteria scores, so that rankings were meaningless. On further research it seems the Copras method is a weighted linear combination of the score for criteria to be maximised divided by the maximum criteria score and score for criteria to be minimised divided by minimum criteria score. It seems that maybe because ADRIA only has maximising criteria this was giving NaN scores (all other methods were found to not error with the same decision matrix and the error occured for any decision matrix with the Copras method). To fix this issue, Copras is removed from the available set of methods in this PR.
All other methods were tested for their outputs to verify that they should not error (given reasonable decision matrix and weightings) and compared to the JMcDM docs.