Fix issue with prices

Author: tsmbland

src/simulation/investment.rs

@@ -11,7 +11,7 @@ use crate::time_slice::{TimeSliceID, TimeSliceInfo};
 use crate::units::{Capacity, Dimensionless, Flow, FlowPerCapacity};
 use anyhow::{Result, ensure};
 use indexmap::IndexMap;
-use itertools::{Itertools, chain, iproduct};
+use itertools::{Itertools, chain};
 use log::{debug, warn};
 use std::collections::{HashMap, VecDeque};
 use std::fmt::Display;
@@ -148,11 +148,6 @@ pub fn perform_agent_investment(
         investment_order.iter().join(" -> ")
     );
 
-    // External prices to be used in dispatch optimisation
-    // Once investments are performed for a commodity, the dispatch system will be able to produce
-    // endogenous prices for that commodity, so we'll gradually remove these external prices.
-    let mut external_prices = prices.clone();
-
     // Keep track of the markets that have been seen so far. This will be used to apply
     // balance constraints in the dispatch optimisation - we only apply balance constraints for
     // commodities that have been seen so far.
@@ -166,7 +161,7 @@ pub fn perform_agent_investment(
             year,
             &net_demand,
             existing_assets,
-            &external_prices,
+            prices,
             &seen_markets,
             &all_selected_assets,
             writer,
@@ -177,18 +172,6 @@ pub fn perform_agent_investment(
             seen_markets.push(market.clone());
         }
 
-        // Remove prices for already-seen markets. Markets which are produced by at
-        // least one asset in the dispatch run will have prices produced endogenously (via the
-        // commodity balance constraints), but markets for which investment has not yet been
-        // performed will, by definition, not have any producers. For these, we provide prices
-        // from the previous dispatch run otherwise they will appear to be free to the model.
-        for (market, time_slice) in iproduct!(
-            investment_set.iter_markets(),
-            model.time_slice_info.iter_ids()
-        ) {
-            external_prices.remove(&market.commodity_id, &market.region_id, time_slice);
-        }
-
         // If no assets have been selected, skip dispatch optimisation
         // **TODO**: this probably means there's no demand for the commodity, which we could
         // presumably preempt
@@ -209,7 +192,7 @@ pub fn perform_agent_investment(
         // their prices using previous values so that they don't appear free
         let solution = DispatchRun::new(model, &all_selected_assets, year)
             .with_market_balance_subset(&seen_markets)
-            .with_input_prices(&external_prices)
+            .with_input_prices(prices)
             .run(&format!("post {investment_set} investment"), writer)?;
 
         // Update demand map with flows from newly added assets

src/simulation/optimisation.rs

@@ -324,23 +324,21 @@ pub fn solve_optimal(model: highs::Model) -> Result<highs::SolvedModel, ModelErr
     }
 }
 
-/// Sanity check for input prices.
-///
-/// Input prices should only be provided for commodities for which there will be no commodity
-/// balance constraint.
-fn check_input_prices(input_prices: &CommodityPrices, markets: &[MarketID]) {
-    let markets_set: HashSet<_> = markets.iter().collect();
-    let has_prices_for_market_subset = input_prices.keys().any(|(commodity_id, region_id, _)| {
-        let market_for_commodity_region = MarketID {
-            commodity_id: commodity_id.clone(),
-            region_id: region_id.clone(),
-        };
-        markets_set.contains(&market_for_commodity_region)
-    });
-    assert!(
-        !has_prices_for_market_subset,
-        "Input prices were included for commodities that are being modelled, which is not allowed."
-    );
+/// Select prices for commodities not being balanced
+fn select_input_prices(
+    input_prices: &CommodityPrices,
+    markets_to_balance: &[MarketID],
+) -> CommodityPrices {
+    let commodity_regions_set: HashSet<(&CommodityID, &RegionID)> = markets_to_balance
+        .iter()
+        .map(|m| (&m.commodity_id, &m.region_id))
+        .collect();
+    input_prices
+        .iter()
+        .filter(|(commodity_id, region_id, _, _)| {
+            !commodity_regions_set.contains(&(commodity_id, region_id))
+        })
+        .collect()
 }
 
 /// Provides the interface for running the dispatch optimisation.
@@ -438,18 +436,25 @@ impl<'model, 'run> DispatchRun<'model, 'run> {
         } else {
             self.markets_to_balance
         };
-        if let Some(input_prices) = self.input_prices {
-            check_input_prices(input_prices, markets_to_balance);
-        }
+
+        // Select prices for commodities not being balanced
+        let input_prices_owned = self
+            .input_prices
+            .map(|prices| select_input_prices(prices, markets_to_balance));
+        let input_prices = input_prices_owned.as_ref();
 
         // Try running dispatch. If it fails because the model is infeasible, it is likely that this
         // is due to unmet demand, in this case, we rerun dispatch including with unmet demand
         // variables for all markets so we can report the offending markets to users
-        match self.run_internal(markets_to_balance, self.markets_to_allow_unmet_demand) {
+        match self.run_internal(
+            markets_to_balance,
+            self.markets_to_allow_unmet_demand,
+            input_prices,
+        ) {
             Ok(solution) => Ok(solution),
             Err(ModelError::NonOptimal(HighsModelStatus::Infeasible)) => {
                 let markets = self
-                    .get_markets_with_unmet_demand(markets_to_balance)
+                    .get_markets_with_unmet_demand(markets_to_balance, input_prices)
                     .expect("Failed to run dispatch to calculate unmet demand");
 
                 ensure!(
@@ -472,9 +477,10 @@ impl<'model, 'run> DispatchRun<'model, 'run> {
     pub fn get_markets_with_unmet_demand(
         &self,
         markets_to_balance: &[MarketID],
+        input_prices: Option<&CommodityPrices>,
     ) -> Result<IndexSet<MarketID>> {
         // Run dispatch including unmet demand variables for all markets being balanced
-        let solution = self.run_internal(markets_to_balance, markets_to_balance)?;
+        let solution = self.run_internal(markets_to_balance, markets_to_balance, input_prices)?;
 
         // Collect markets with unmet demand
         Ok(solution.get_markets_with_unmet_demand())
@@ -485,13 +491,14 @@ impl<'model, 'run> DispatchRun<'model, 'run> {
         &self,
         markets_to_balance: &[MarketID],
         markets_to_allow_unmet_demand: &[MarketID],
+        input_prices: Option<&CommodityPrices>,
     ) -> Result<Solution<'model>, ModelError> {
         // Set up problem
         let mut problem = Problem::default();
         let mut variables = VariableMap::new_with_asset_vars(
             &mut problem,
             self.model,
-            self.input_prices,
+            input_prices,
             self.existing_assets,
             self.candidate_assets,
             self.year,