Turn off the ironing out loop by default

docs/release_notes/upcoming.md

@@ -31,6 +31,8 @@ ready to be released, carry out the following steps:
 - Allow for adding both a `prod` and `cons` levy to a commodity ([#969])
 - Availability limits can now be provided at multiple levels for a process ([#1018])
 - Pricing strategy can now vary by commodity ([#1021])
+- `marginal` and `full` commodity pricing strategies no longer require enabling
+  `please_give_me_broken_results` ([#1185])
 
 ## Experimental features
 

schemas/input/model.yaml

@@ -31,7 +31,7 @@ properties:
   max_ironing_out_iterations:
     type: integer
     description: The maximum number of iterations to run the "ironing out" step of agent investment for
-    default: 10
+    default: 1
   price_tolerance:
     type: number
     description: The relative tolerance for price convergence in the ironing out loop

src/cli/example.rs

@@ -100,7 +100,7 @@ fn handle_example_extract_command(name: &str, dest: Option<&Path>, patch: bool)
 /// If `patch` is `true`, then the corresponding patched example will be extracted.
 fn extract_example(name: &str, patch: bool, dest: &Path) -> Result<()> {
     if patch {
-        let patches = get_patches(name)?;
+        let (file_patches, toml_patch) = get_patches(name)?;
 
         // NB: All patched models are based on `simple`, for now
         let example = Example::from_name("simple").unwrap();
@@ -114,10 +114,12 @@ fn extract_example(name: &str, patch: bool, dest: &Path) -> Result<()> {
 
         // Patch example and put contents in dest
         fs::create_dir(dest).context("Could not create output directory")?;
-        ModelPatch::new(example_path)
-            .with_file_patches(patches.to_owned())
-            .build(dest)
-            .context("Failed to patch example")
+        let mut model_patch =
+            ModelPatch::new(example_path).with_file_patches(file_patches.to_owned());
+        if let Some(toml_patch) = toml_patch.as_ref() {
+            model_patch = model_patch.with_toml_patch(toml_patch);
+        }
+        model_patch.build(dest).context("Failed to patch example")
     } else {
         // Otherwise it's just a regular example
         let example = Example::from_name(name)?;

src/example/patches.rs

@@ -1,36 +1,77 @@
-//! File patches to be used in integration tests.
+//! Patches to be used in integration tests.
 //!
 //! This is used to test small variations on existing example models.
 use crate::patch::FilePatch;
 use anyhow::{Context, Result};
 use std::{collections::BTreeMap, sync::LazyLock};
 
-/// A map of file patches, keyed by name
-type PatchMap = BTreeMap<&'static str, Vec<FilePatch>>;
+/// Map of patches keyed by name, with the file patches and an optional TOML patch
+type PatchMap = BTreeMap<&'static str, (Vec<FilePatch>, Option<&'static str>)>;
 
-/// The file patches, keyed by name
+/// The patches, keyed by name
 static PATCHES: LazyLock<PatchMap> = LazyLock::new(get_all_patches);
 
 /// Get all patches
 fn get_all_patches() -> PatchMap {
     [
+        // The simple example with gas boiler process made divisible
         (
-            // The simple example with gas boiler process made divisible
             "simple_divisible",
-            vec![
-                FilePatch::new("processes.csv")
-                    .with_deletion("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,")
-                    .with_addition("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,1000"),
-            ],
+            (
+                vec![
+                    FilePatch::new("processes.csv")
+                        .with_deletion("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,")
+                        .with_addition("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,1000"),
+                ],
+                None,
+            ),
         ),
         // The simple example with objective type set to NPV for one agent
         (
             "simple_npv",
-            vec![
-                FilePatch::new("agent_objectives.csv")
-                    .with_deletion("A0_RES,all,lcox,,")
-                    .with_addition("A0_RES,all,npv,,"),
-            ],
+            (
+                vec![
+                    FilePatch::new("agent_objectives.csv")
+                        .with_deletion("A0_RES,all,lcox,,")
+                        .with_addition("A0_RES,all,npv,,"),
+                ],
+                None,
+            ),
+        ),
+        (
+            // The simple example with electricity priced using marginal costs
+            "simple_marginal",
+            (
+                vec![FilePatch::new("commodities.csv").with_replacement(&[
+                    "id,description,type,time_slice_level,pricing_strategy,units",
+                    "GASPRD,Gas produced,sed,season,shadow,PJ",
+                    "GASNAT,Natural gas,sed,season,shadow,PJ",
+                    "ELCTRI,Electricity,sed,daynight,marginal,PJ",
+                    "RSHEAT,Residential heating,svd,daynight,shadow,PJ",
+                    "CO2EMT,CO2 emitted,oth,annual,unpriced,ktCO2",
+                ])],
+                None,
+            ),
+        ),
+        (
+            // The simple example with gas commodities priced using full costs
+            "simple_full",
+            (
+                vec![FilePatch::new("commodities.csv").with_replacement(&[
+                    "id,description,type,time_slice_level,pricing_strategy,units",
+                    "GASPRD,Gas produced,sed,season,full,PJ",
+                    "GASNAT,Natural gas,sed,season,full,PJ",
+                    "ELCTRI,Electricity,sed,daynight,shadow,PJ",
+                    "RSHEAT,Residential heating,svd,daynight,shadow,PJ",
+                    "CO2EMT,CO2 emitted,oth,annual,unpriced,ktCO2",
+                ])],
+                None,
+            ),
+        ),
+        // The simple example with the ironing-out loop turned on
+        (
+            "simple_ironing_out",
+            (vec![], Some("max_ironing_out_iterations = 10")),
         ),
     ]
     .into_iter()
@@ -43,8 +84,8 @@ pub fn get_patch_names() -> impl Iterator<Item = &'static str> {
 }
 
 /// Get patches for the named patched example
-pub fn get_patches(name: &str) -> Result<&[FilePatch]> {
-    Ok(PATCHES
+pub fn get_patches(name: &str) -> Result<&'static (Vec<FilePatch>, Option<&'static str>)> {
+    PATCHES
         .get(name)
-        .with_context(|| format!("Patched example '{name}' not found"))?)
+        .with_context(|| format!("Patched example '{name}' not found"))
 }

src/input/commodity.rs

@@ -164,18 +164,13 @@ fn validate_commodity(commodity: &Commodity) -> Result<()> {
         }
     }
 
-    // Gatekeep alternative pricing options
-    if !matches!(
-        commodity.pricing_strategy,
-        PricingStrategy::Shadow | PricingStrategy::Unpriced
-    ) {
+    // Gatekeep scarcity-adjusted pricing option
+    if commodity.pricing_strategy == PricingStrategy::ScarcityAdjusted {
         ensure!(
             broken_model_options_allowed(),
-            "Price strategies other than 'shadow' and 'unpriced' are currently experimental. \
+            "The 'scarcity' pricing strategy is currently experimental. \
             To run anyway, set the {ALLOW_BROKEN_OPTION_NAME} option to true."
         );
-    }
-    if commodity.pricing_strategy == PricingStrategy::ScarcityAdjusted {
         warn!(
             "The pricing strategy for {} is set to 'scarcity'. Commodity prices may be \
             incorrect if assets have more than one output commodity. See: {ISSUES_URL}/677",

src/model/parameters.rs

@@ -77,7 +77,7 @@ define_unit_param_default!(default_capacity_limit_factor, Dimensionless, 0.1);
 define_unit_param_default!(default_value_of_lost_load, MoneyPerFlow, 1e9);
 define_unit_param_default!(default_price_tolerance, Dimensionless, 1e-6);
 define_unit_param_default!(default_remaining_demand_absolute_tolerance, Flow, 1e-12);
-define_param_default!(default_max_ironing_out_iterations, u32, 10);
+define_param_default!(default_max_ironing_out_iterations, u32, 1);
 define_param_default!(default_capacity_margin, f64, 0.2);
 define_param_default!(default_mothball_years, u32, 0);
 

src/patch.rs

@@ -120,6 +120,8 @@ pub struct FilePatch {
     filename: String,
     /// The header row (optional). If `None`, the header is not checked against base files.
     header_row: Option<Vec<String>>,
+    /// Full replacement content for this file (optional)
+    replacement_content: Option<String>,
     /// Rows to delete (each row is a vector of fields)
     to_delete: CSVTable,
     /// Rows to add (each row is a vector of fields)
@@ -132,13 +134,18 @@ impl FilePatch {
         FilePatch {
             filename: filename.into(),
             header_row: None,
+            replacement_content: None,
             to_delete: IndexSet::new(),
             to_add: IndexSet::new(),
         }
     }
 
     /// Set the header row for this patch (header should be a comma-joined string, e.g. "a,b,c").
     pub fn with_header(mut self, header: impl Into<String>) -> Self {
+        assert!(
+            self.replacement_content.is_none(),
+            "Cannot set header when replacement content is set for this FilePatch",
+        );
         assert!(
             self.header_row.is_none(),
             "Header already set for this FilePatch",
@@ -149,8 +156,48 @@ impl FilePatch {
         self
     }
 
+    /// Set full replacement content for this file from a slice of lines.
+    ///
+    /// Each line is joined with newlines, and a trailing newline is added.
+    /// All lines must have the same number of columns (commas).
+    /// Example: `with_replacement(&["header1,header2", "value1,value2"])`
+    pub fn with_replacement(mut self, lines: &[&str]) -> Self {
+        assert!(
+            self.header_row.is_none(),
+            "Cannot set replacement content when header is set for this FilePatch",
+        );
+        assert!(
+            self.to_delete.is_empty() && self.to_add.is_empty(),
+            "Cannot set replacement content when additions/deletions are set for this FilePatch",
+        );
+        assert!(
+            self.replacement_content.is_none(),
+            "Replacement content already set for this FilePatch",
+        );
+
+        // Validate that all lines have the same number of columns
+        if !lines.is_empty() {
+            let first_col_count = lines[0].matches(',').count() + 1;
+            for (idx, line) in lines.iter().enumerate() {
+                let col_count = line.matches(',').count() + 1;
+                assert_eq!(
+                    col_count, first_col_count,
+                    "Line {idx} has {col_count} columns but line 0 has {first_col_count}: {line:?}"
+                );
+            }
+        }
+
+        let content = lines.join("\n") + "\n";
+        self.replacement_content = Some(content);
+        self
+    }
+
     /// Add a row to the patch (row should be a comma-joined string, e.g. "a,b,c").
     pub fn with_addition(mut self, row: impl Into<String>) -> Self {
+        assert!(
+            self.replacement_content.is_none(),
+            "Cannot add rows when replacement content is set for this FilePatch",
+        );
         let s = row.into();
         let v = s.split(',').map(|s| s.trim().to_string()).collect();
         self.to_add.insert(v);
@@ -159,6 +206,10 @@ impl FilePatch {
 
     /// Mark a row for deletion from the base (row should be a comma-joined string, e.g. "a,b,c").
     pub fn with_deletion(mut self, row: impl Into<String>) -> Self {
+        assert!(
+            self.replacement_content.is_none(),
+            "Cannot delete rows when replacement content is set for this FilePatch",
+        );
         let s = row.into();
         let v = s.split(',').map(|s| s.trim().to_string()).collect();
         self.to_delete.insert(v);
@@ -167,13 +218,21 @@ impl FilePatch {
 
     /// Apply this patch to a base model and return the modified CSV as a string.
     fn apply(&self, base_model_dir: &Path) -> Result<String> {
-        // Read the base file to string
+        // Read and validate the base file path
         let base_path = base_model_dir.join(&self.filename);
         ensure!(
             base_path.exists() && base_path.is_file(),
             "Base file for patching does not exist: {}",
             base_path.display()
         );
+
+        // If this patch is a full replacement, validate the base file exists
+        // (checked above) and return the replacement content
+        if let Some(content) = &self.replacement_content {
+            return Ok(content.clone());
+        }
+
+        // Read the base file to string
         let base = fs::read_to_string(&base_path)?;
 
         // Apply the patch
@@ -232,7 +291,6 @@ fn modify_base_with_patch(base: &str, patch: &FilePatch) -> Result<String> {
             header_row_vec.join(", ")
         );
     }
-
     // Read all rows from the base, preserving order and checking for duplicates
     let mut base_rows: CSVTable = CSVTable::new();
     for result in reader.records() {
@@ -278,6 +336,16 @@ fn modify_base_with_patch(base: &str, patch: &FilePatch) -> Result<String> {
         );
     }
 
+    // Check all rows match base header length
+    let expected_len = base_header_vec.len();
+    for row in &base_rows {
+        ensure!(
+            row.len() == expected_len,
+            "Row has {} columns but header has {expected_len}: {row:?}",
+            row.len(),
+        );
+    }
+
     // Serialize CSV output using csv::Writer
     let mut wtr = Writer::from_writer(vec![]);
     wtr.write_record(base_header_vec.iter())?;
@@ -379,6 +447,73 @@ mod tests {
         assert!(assets_content.contains("GASDRV,GBR,A0_GEX,4003.26,2020"));
     }
 
+    #[test]
+    fn file_patch_with_replacement() {
+        let expected = "col1,col2\nnew1,new2\n";
+
+        let model_dir = ModelPatch::from_example("simple")
+            .with_file_patch(
+                FilePatch::new("assets.csv").with_replacement(&["col1,col2", "new1,new2"]),
+            )
+            .build_to_tempdir()
+            .unwrap();
+
+        let assets_path = model_dir.path().join("assets.csv");
+        let assets_content = std::fs::read_to_string(assets_path).unwrap();
+        assert_eq!(assets_content, expected);
+    }
+
+    #[test]
+    #[should_panic(
+        expected = "Cannot set replacement content when header is set for this FilePatch"
+    )]
+    fn file_patch_replacement_after_header_panics() {
+        let _ = FilePatch::new("assets.csv")
+            .with_header("col1,col2")
+            .with_replacement(&["col1,col2", "a,b"]);
+    }
+
+    #[test]
+    #[should_panic(
+        expected = "Cannot set replacement content when additions/deletions are set for this FilePatch"
+    )]
+    fn file_patch_replacement_after_addition_panics() {
+        let _ = FilePatch::new("assets.csv")
+            .with_addition("a,b")
+            .with_replacement(&["col1,col2", "a,b"]);
+    }
+
+    #[test]
+    #[should_panic(expected = "Cannot add rows when replacement content is set for this FilePatch")]
+    fn file_patch_addition_after_replacement_panics() {
+        let _ = FilePatch::new("assets.csv")
+            .with_replacement(&["col1,col2", "a,b"])
+            .with_addition("c,d");
+    }
+
+    #[test]
+    fn file_patch_with_replacement_missing_base_file_fails() {
+        let model_patch = ModelPatch::from_example("simple").with_file_patch(
+            FilePatch::new("not_a_real_file.csv").with_replacement(&["x,y", "1,2"]),
+        );
+
+        let expected = format!(
+            "Base file for patching does not exist: {}",
+            std::path::PathBuf::from("examples")
+                .join("simple")
+                .join("not_a_real_file.csv")
+                .display()
+        );
+
+        assert_error!(model_patch.build_to_tempdir(), expected);
+    }
+
+    #[test]
+    #[should_panic(expected = "Line 1 has 2 columns but line 0 has 3")]
+    fn file_patch_replacement_column_count_mismatch_panics() {
+        let _ = FilePatch::new("test.csv").with_replacement(&["col1,col2,col3", "a,b"]);
+    }
+
     #[test]
     fn toml_patch() {
         // Patch to add an extra milestone year (2050)