From fbc8be166e01403c5e2ed52ba7bdeece5ef94182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Tue, 30 Apr 2024 19:59:57 -0400 Subject: [PATCH 1/2] fix: Avoid NPE when computing suggestions for improvements When providers are located in an area outside of the friction data (most commonly in a coast) but still fall inside the region contour (because it has better resolution), the engine will fail to compute any demand information for it. This lead to and NPE in the suggestions algorithm when computing the relative cost of such providers. --- src/planwise/component/engine.clj | 4 +- src/planwise/component/scenarios.clj | 13 +++- src/planwise/engine/suggestions.clj | 103 ++++++++++++++------------- 3 files changed, 67 insertions(+), 53 deletions(-) diff --git a/src/planwise/component/engine.clj b/src/planwise/component/engine.clj index 0cf40e06..45c3351a 100644 --- a/src/planwise/component/engine.clj +++ b/src/planwise/component/engine.clj @@ -148,7 +148,9 @@ (filter some? (map assoc-raster-path providers coverages)))) (defn- providers-in-project-with-coverage - "Returns the providers relevant for this project with their coverages already resolved" + "Returns the providers relevant for this project with their coverages already + resolved. NOTE: This function discards any provider for which we cannot compute a + coverage raster." [engine project] (let [providers (common/providers-in-project (:providers-set engine) project)] (query-provider-coverages engine project providers))) diff --git a/src/planwise/component/scenarios.clj b/src/planwise/component/scenarios.clj index 5506533d..573fe8fd 100644 --- a/src/planwise/component/scenarios.clj +++ b/src/planwise/component/scenarios.clj @@ -522,4 +522,15 @@ (def changes (subvec providers-and-changes (count initial-providers))) (def capacity-sat (Math/abs (- initial-demand final-demand))) - (>= (reduce + (mapv :satisfied changes)) capacity-sat)) + (>= (reduce + (mapv :satisfied changes)) capacity-sat) + + ;; Testing issue #727 + (let [store (:planwise.component/scenarios integrant.repl.state/system) + project-store (:planwise.component/projects2 integrant.repl.state/system) + project (planwise.boundary.projects2/get-project project-store 40) + scenario (get-scenario store 128) + engine (:planwise.component/engine integrant.repl.state/system) + settings {:analysis-type "action", :available-budget 0, :no-action-costs true}] + (get-suggestions-for-improving-providers store project scenario)) + + ) diff --git a/src/planwise/engine/suggestions.clj b/src/planwise/engine/suggestions.clj index 1ec14681..60836fcf 100644 --- a/src/planwise/engine/suggestions.clj +++ b/src/planwise/engine/suggestions.clj @@ -7,11 +7,12 @@ [planwise.engine.common :as common] [planwise.component.coverage.rasterize :as rasterize] [planwise.engine.demand :as demand] + [planwise.util.collections :refer [find-by]] [planwise.util.files :as files] [planwise.util.geo :as geo] + [planwise.common :as c] [clojure.set :refer [rename-keys]] - [taoensso.timbre :as timbre] - [planwise.common :as c]) + [taoensso.timbre :as timbre]) (:import [planwise.engine Algorithm])) (timbre/refer-timbre) @@ -369,77 +370,77 @@ (defn get-provider-capacity-and-cost [provider settings] - (let [max-capacity (:max-capacity settings) - action-capacity (if max-capacity - (min (:required-capacity provider) (:max-capacity settings)) - (:required-capacity provider)) - action-cost (get-increasing-cost - {:action (if (:applicable? provider) + (let [max-capacity (:max-capacity settings) + action-capacity (if max-capacity + (min (:required-capacity provider) (:max-capacity settings)) + (:required-capacity provider)) + action-cost (get-increasing-cost + {:action (if (:applicable? provider) "increase-provider" "upgrade-provider") - :capacity action-capacity} - settings)] + :capacity action-capacity} + settings) + ratio (if-not (zero? action-cost) + (/ action-capacity action-cost) + 0)] (cond (< (:available-budget settings) action-cost) nil - :else {:action-capacity action-capacity - :action-cost action-cost}))) + :else {:action-capacity action-capacity + :action-cost action-cost + :ratio ratio}))) -(defn insert-in-sorted-coll +(defn- insert-in-sorted-coll [coll value criteria] (sort-by criteria > (conj coll value))) -(defn get-information-from-demand +(defn- get-information-from-demand [all-providers id keys] - (select-keys - (first (filter #(= id (:id %)) all-providers)) - keys)) + (-> all-providers + (find-by :id id) + (select-keys keys))) (defn get-provider-capacity-and-unsatisfied-demand [{:keys [unsatisfied-demand required-capacity] :as provider} {:keys [max-capacity] :as settings}] - (let [action-capacity (if max-capacity - (min required-capacity max-capacity) - required-capacity) - action-cost (get-increasing-cost - {:action (if (:applicable? provider) - "increase-provider" - "upgrade-provider") - :capacity action-capacity} - settings)] - {:unsatisfied-demand unsatisfied-demand + (let [required-capacity (or required-capacity 0) + action-capacity (if max-capacity + (min required-capacity max-capacity) + required-capacity) + action-cost (get-increasing-cost + {:action (if (:applicable? provider) + "increase-provider" + "upgrade-provider") + :capacity action-capacity} + settings)] + ;; We provide 0 default values to handle edge cases when the provider has no + ;; previous data computed for the scenario. + {:unsatisfied-demand (or unsatisfied-demand 0) :action-capacity action-capacity :action-cost action-cost})) (defn- provider-with-data [provider providers-data {:keys [analysis-type] :as settings}] - (let [budget? (c/is-budget analysis-type)] - (when-let [intervention ((if budget? get-provider-capacity-and-cost get-provider-capacity-and-unsatisfied-demand) - (merge - provider - (get-information-from-demand - providers-data - (:id provider) - (if budget? [:required-capacity] [:required-capacity :unsatisfied-demand]))) - settings)] - (merge - provider - intervention - (if budget? - (let [{:keys [action-capacity action-cost]} intervention] - {:ratio (if (not (zero? action-cost)) - (/ action-capacity action-cost) - 0)})))))) + (let [budget? (c/is-budget analysis-type) + intervention-builder (if budget? get-provider-capacity-and-cost get-provider-capacity-and-unsatisfied-demand) + requirements (get-information-from-demand providers-data + (:id provider) + [:required-capacity :unsatisfied-demand])] + (when (seq requirements) + (when-let [intervention (intervention-builder (merge provider requirements) settings)] + (merge provider intervention))))) (defn get-sorted-providers-interventions [engine project {:keys [providers-data changeset] :as scenario} {:keys [analysis-type] :as settings}] - (let [{:keys [engine-config config provider-set-id region-id coverage-algorithm]} project - providers-collection (common/providers-in-project (:providers-set engine) project) - sort-key (if (c/is-budget analysis-type) :ratio :unsatisfied-demand)] + (let [{:keys [engine-config + provider-set-id + region-id + coverage-algorithm]} project + providers-collection (common/providers-in-project (:providers-set engine) project) + sort-key (if (c/is-budget analysis-type) :ratio :unsatisfied-demand)] (reduce (fn [suggestions provider] - (insert-in-sorted-coll - suggestions - (provider-with-data provider providers-data settings) - sort-key)) + (if-let [suggestion (provider-with-data provider providers-data settings)] + (insert-in-sorted-coll suggestions suggestion sort-key) + suggestions)) [] providers-collection))) From 212d7d05694895384d73254e42fd51430b95bf71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Tue, 30 Apr 2024 20:08:51 -0400 Subject: [PATCH 2/2] chore: Fix formatting --- src/planwise/component/scenarios.clj | 4 +--- src/planwise/engine/suggestions.clj | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/planwise/component/scenarios.clj b/src/planwise/component/scenarios.clj index 573fe8fd..20a100a1 100644 --- a/src/planwise/component/scenarios.clj +++ b/src/planwise/component/scenarios.clj @@ -531,6 +531,4 @@ scenario (get-scenario store 128) engine (:planwise.component/engine integrant.repl.state/system) settings {:analysis-type "action", :available-budget 0, :no-action-costs true}] - (get-suggestions-for-improving-providers store project scenario)) - - ) + (get-suggestions-for-improving-providers store project scenario))) diff --git a/src/planwise/engine/suggestions.clj b/src/planwise/engine/suggestions.clj index 60836fcf..61be16a9 100644 --- a/src/planwise/engine/suggestions.clj +++ b/src/planwise/engine/suggestions.clj @@ -422,8 +422,8 @@ (let [budget? (c/is-budget analysis-type) intervention-builder (if budget? get-provider-capacity-and-cost get-provider-capacity-and-unsatisfied-demand) requirements (get-information-from-demand providers-data - (:id provider) - [:required-capacity :unsatisfied-demand])] + (:id provider) + [:required-capacity :unsatisfied-demand])] (when (seq requirements) (when-let [intervention (intervention-builder (merge provider requirements) settings)] (merge provider intervention)))))