Skip to content

Commit 5389d44

Browse files
authored
Merge pull request #1170 from EnergySystemsModellingLab/ironing_out_off
Turn off the ironing out loop by default
2 parents 3484b05 + a0c9e2d commit 5389d44

37 files changed

Lines changed: 6343 additions & 5463 deletions

docs/release_notes/upcoming.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ ready to be released, carry out the following steps:
3131
- Allow for adding both a `prod` and `cons` levy to a commodity ([#969])
3232
- Availability limits can now be provided at multiple levels for a process ([#1018])
3333
- Pricing strategy can now vary by commodity ([#1021])
34+
- `marginal` and `full` commodity pricing strategies no longer require enabling
35+
`please_give_me_broken_results` ([#1185])
3436

3537
## Experimental features
3638

schemas/input/model.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ properties:
3131
max_ironing_out_iterations:
3232
type: integer
3333
description: The maximum number of iterations to run the "ironing out" step of agent investment for
34-
default: 10
34+
default: 1
3535
price_tolerance:
3636
type: number
3737
description: The relative tolerance for price convergence in the ironing out loop

src/cli/example.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ fn handle_example_extract_command(name: &str, dest: Option<&Path>, patch: bool)
100100
/// If `patch` is `true`, then the corresponding patched example will be extracted.
101101
fn extract_example(name: &str, patch: bool, dest: &Path) -> Result<()> {
102102
if patch {
103-
let patches = get_patches(name)?;
103+
let (file_patches, toml_patch) = get_patches(name)?;
104104

105105
// NB: All patched models are based on `simple`, for now
106106
let example = Example::from_name("simple").unwrap();
@@ -114,10 +114,12 @@ fn extract_example(name: &str, patch: bool, dest: &Path) -> Result<()> {
114114

115115
// Patch example and put contents in dest
116116
fs::create_dir(dest).context("Could not create output directory")?;
117-
ModelPatch::new(example_path)
118-
.with_file_patches(patches.to_owned())
119-
.build(dest)
120-
.context("Failed to patch example")
117+
let mut model_patch =
118+
ModelPatch::new(example_path).with_file_patches(file_patches.to_owned());
119+
if let Some(toml_patch) = toml_patch.as_ref() {
120+
model_patch = model_patch.with_toml_patch(toml_patch);
121+
}
122+
model_patch.build(dest).context("Failed to patch example")
121123
} else {
122124
// Otherwise it's just a regular example
123125
let example = Example::from_name(name)?;

src/example/patches.rs

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,77 @@
1-
//! File patches to be used in integration tests.
1+
//! Patches to be used in integration tests.
22
//!
33
//! This is used to test small variations on existing example models.
44
use crate::patch::FilePatch;
55
use anyhow::{Context, Result};
66
use std::{collections::BTreeMap, sync::LazyLock};
77

8-
/// A map of file patches, keyed by name
9-
type PatchMap = BTreeMap<&'static str, Vec<FilePatch>>;
8+
/// Map of patches keyed by name, with the file patches and an optional TOML patch
9+
type PatchMap = BTreeMap<&'static str, (Vec<FilePatch>, Option<&'static str>)>;
1010

11-
/// The file patches, keyed by name
11+
/// The patches, keyed by name
1212
static PATCHES: LazyLock<PatchMap> = LazyLock::new(get_all_patches);
1313

1414
/// Get all patches
1515
fn get_all_patches() -> PatchMap {
1616
[
17+
// The simple example with gas boiler process made divisible
1718
(
18-
// The simple example with gas boiler process made divisible
1919
"simple_divisible",
20-
vec![
21-
FilePatch::new("processes.csv")
22-
.with_deletion("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,")
23-
.with_addition("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,1000"),
24-
],
20+
(
21+
vec![
22+
FilePatch::new("processes.csv")
23+
.with_deletion("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,")
24+
.with_addition("RGASBR,Gas boiler,all,RSHEAT,2020,2040,1.0,1000"),
25+
],
26+
None,
27+
),
2528
),
2629
// The simple example with objective type set to NPV for one agent
2730
(
2831
"simple_npv",
29-
vec![
30-
FilePatch::new("agent_objectives.csv")
31-
.with_deletion("A0_RES,all,lcox,,")
32-
.with_addition("A0_RES,all,npv,,"),
33-
],
32+
(
33+
vec![
34+
FilePatch::new("agent_objectives.csv")
35+
.with_deletion("A0_RES,all,lcox,,")
36+
.with_addition("A0_RES,all,npv,,"),
37+
],
38+
None,
39+
),
40+
),
41+
(
42+
// The simple example with electricity priced using marginal costs
43+
"simple_marginal",
44+
(
45+
vec![FilePatch::new("commodities.csv").with_replacement(&[
46+
"id,description,type,time_slice_level,pricing_strategy,units",
47+
"GASPRD,Gas produced,sed,season,shadow,PJ",
48+
"GASNAT,Natural gas,sed,season,shadow,PJ",
49+
"ELCTRI,Electricity,sed,daynight,marginal,PJ",
50+
"RSHEAT,Residential heating,svd,daynight,shadow,PJ",
51+
"CO2EMT,CO2 emitted,oth,annual,unpriced,ktCO2",
52+
])],
53+
None,
54+
),
55+
),
56+
(
57+
// The simple example with gas commodities priced using full costs
58+
"simple_full",
59+
(
60+
vec![FilePatch::new("commodities.csv").with_replacement(&[
61+
"id,description,type,time_slice_level,pricing_strategy,units",
62+
"GASPRD,Gas produced,sed,season,full,PJ",
63+
"GASNAT,Natural gas,sed,season,full,PJ",
64+
"ELCTRI,Electricity,sed,daynight,shadow,PJ",
65+
"RSHEAT,Residential heating,svd,daynight,shadow,PJ",
66+
"CO2EMT,CO2 emitted,oth,annual,unpriced,ktCO2",
67+
])],
68+
None,
69+
),
70+
),
71+
// The simple example with the ironing-out loop turned on
72+
(
73+
"simple_ironing_out",
74+
(vec![], Some("max_ironing_out_iterations = 10")),
3475
),
3576
]
3677
.into_iter()
@@ -43,8 +84,8 @@ pub fn get_patch_names() -> impl Iterator<Item = &'static str> {
4384
}
4485

4586
/// Get patches for the named patched example
46-
pub fn get_patches(name: &str) -> Result<&[FilePatch]> {
47-
Ok(PATCHES
87+
pub fn get_patches(name: &str) -> Result<&'static (Vec<FilePatch>, Option<&'static str>)> {
88+
PATCHES
4889
.get(name)
49-
.with_context(|| format!("Patched example '{name}' not found"))?)
90+
.with_context(|| format!("Patched example '{name}' not found"))
5091
}

src/input/commodity.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -164,18 +164,13 @@ fn validate_commodity(commodity: &Commodity) -> Result<()> {
164164
}
165165
}
166166

167-
// Gatekeep alternative pricing options
168-
if !matches!(
169-
commodity.pricing_strategy,
170-
PricingStrategy::Shadow | PricingStrategy::Unpriced
171-
) {
167+
// Gatekeep scarcity-adjusted pricing option
168+
if commodity.pricing_strategy == PricingStrategy::ScarcityAdjusted {
172169
ensure!(
173170
broken_model_options_allowed(),
174-
"Price strategies other than 'shadow' and 'unpriced' are currently experimental. \
171+
"The 'scarcity' pricing strategy is currently experimental. \
175172
To run anyway, set the {ALLOW_BROKEN_OPTION_NAME} option to true."
176173
);
177-
}
178-
if commodity.pricing_strategy == PricingStrategy::ScarcityAdjusted {
179174
warn!(
180175
"The pricing strategy for {} is set to 'scarcity'. Commodity prices may be \
181176
incorrect if assets have more than one output commodity. See: {ISSUES_URL}/677",

src/model/parameters.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ define_unit_param_default!(default_capacity_limit_factor, Dimensionless, 0.1);
7777
define_unit_param_default!(default_value_of_lost_load, MoneyPerFlow, 1e9);
7878
define_unit_param_default!(default_price_tolerance, Dimensionless, 1e-6);
7979
define_unit_param_default!(default_remaining_demand_absolute_tolerance, Flow, 1e-12);
80-
define_param_default!(default_max_ironing_out_iterations, u32, 10);
80+
define_param_default!(default_max_ironing_out_iterations, u32, 1);
8181
define_param_default!(default_capacity_margin, f64, 0.2);
8282
define_param_default!(default_mothball_years, u32, 0);
8383

src/patch.rs

Lines changed: 137 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ pub struct FilePatch {
120120
filename: String,
121121
/// The header row (optional). If `None`, the header is not checked against base files.
122122
header_row: Option<Vec<String>>,
123+
/// Full replacement content for this file (optional)
124+
replacement_content: Option<String>,
123125
/// Rows to delete (each row is a vector of fields)
124126
to_delete: CSVTable,
125127
/// Rows to add (each row is a vector of fields)
@@ -132,13 +134,18 @@ impl FilePatch {
132134
FilePatch {
133135
filename: filename.into(),
134136
header_row: None,
137+
replacement_content: None,
135138
to_delete: IndexSet::new(),
136139
to_add: IndexSet::new(),
137140
}
138141
}
139142

140143
/// Set the header row for this patch (header should be a comma-joined string, e.g. "a,b,c").
141144
pub fn with_header(mut self, header: impl Into<String>) -> Self {
145+
assert!(
146+
self.replacement_content.is_none(),
147+
"Cannot set header when replacement content is set for this FilePatch",
148+
);
142149
assert!(
143150
self.header_row.is_none(),
144151
"Header already set for this FilePatch",
@@ -149,8 +156,48 @@ impl FilePatch {
149156
self
150157
}
151158

159+
/// Set full replacement content for this file from a slice of lines.
160+
///
161+
/// Each line is joined with newlines, and a trailing newline is added.
162+
/// All lines must have the same number of columns (commas).
163+
/// Example: `with_replacement(&["header1,header2", "value1,value2"])`
164+
pub fn with_replacement(mut self, lines: &[&str]) -> Self {
165+
assert!(
166+
self.header_row.is_none(),
167+
"Cannot set replacement content when header is set for this FilePatch",
168+
);
169+
assert!(
170+
self.to_delete.is_empty() && self.to_add.is_empty(),
171+
"Cannot set replacement content when additions/deletions are set for this FilePatch",
172+
);
173+
assert!(
174+
self.replacement_content.is_none(),
175+
"Replacement content already set for this FilePatch",
176+
);
177+
178+
// Validate that all lines have the same number of columns
179+
if !lines.is_empty() {
180+
let first_col_count = lines[0].matches(',').count() + 1;
181+
for (idx, line) in lines.iter().enumerate() {
182+
let col_count = line.matches(',').count() + 1;
183+
assert_eq!(
184+
col_count, first_col_count,
185+
"Line {idx} has {col_count} columns but line 0 has {first_col_count}: {line:?}"
186+
);
187+
}
188+
}
189+
190+
let content = lines.join("\n") + "\n";
191+
self.replacement_content = Some(content);
192+
self
193+
}
194+
152195
/// Add a row to the patch (row should be a comma-joined string, e.g. "a,b,c").
153196
pub fn with_addition(mut self, row: impl Into<String>) -> Self {
197+
assert!(
198+
self.replacement_content.is_none(),
199+
"Cannot add rows when replacement content is set for this FilePatch",
200+
);
154201
let s = row.into();
155202
let v = s.split(',').map(|s| s.trim().to_string()).collect();
156203
self.to_add.insert(v);
@@ -159,6 +206,10 @@ impl FilePatch {
159206

160207
/// Mark a row for deletion from the base (row should be a comma-joined string, e.g. "a,b,c").
161208
pub fn with_deletion(mut self, row: impl Into<String>) -> Self {
209+
assert!(
210+
self.replacement_content.is_none(),
211+
"Cannot delete rows when replacement content is set for this FilePatch",
212+
);
162213
let s = row.into();
163214
let v = s.split(',').map(|s| s.trim().to_string()).collect();
164215
self.to_delete.insert(v);
@@ -167,13 +218,21 @@ impl FilePatch {
167218

168219
/// Apply this patch to a base model and return the modified CSV as a string.
169220
fn apply(&self, base_model_dir: &Path) -> Result<String> {
170-
// Read the base file to string
221+
// Read and validate the base file path
171222
let base_path = base_model_dir.join(&self.filename);
172223
ensure!(
173224
base_path.exists() && base_path.is_file(),
174225
"Base file for patching does not exist: {}",
175226
base_path.display()
176227
);
228+
229+
// If this patch is a full replacement, validate the base file exists
230+
// (checked above) and return the replacement content
231+
if let Some(content) = &self.replacement_content {
232+
return Ok(content.clone());
233+
}
234+
235+
// Read the base file to string
177236
let base = fs::read_to_string(&base_path)?;
178237

179238
// Apply the patch
@@ -232,7 +291,6 @@ fn modify_base_with_patch(base: &str, patch: &FilePatch) -> Result<String> {
232291
header_row_vec.join(", ")
233292
);
234293
}
235-
236294
// Read all rows from the base, preserving order and checking for duplicates
237295
let mut base_rows: CSVTable = CSVTable::new();
238296
for result in reader.records() {
@@ -278,6 +336,16 @@ fn modify_base_with_patch(base: &str, patch: &FilePatch) -> Result<String> {
278336
);
279337
}
280338

339+
// Check all rows match base header length
340+
let expected_len = base_header_vec.len();
341+
for row in &base_rows {
342+
ensure!(
343+
row.len() == expected_len,
344+
"Row has {} columns but header has {expected_len}: {row:?}",
345+
row.len(),
346+
);
347+
}
348+
281349
// Serialize CSV output using csv::Writer
282350
let mut wtr = Writer::from_writer(vec![]);
283351
wtr.write_record(base_header_vec.iter())?;
@@ -379,6 +447,73 @@ mod tests {
379447
assert!(assets_content.contains("GASDRV,GBR,A0_GEX,4003.26,2020"));
380448
}
381449

450+
#[test]
451+
fn file_patch_with_replacement() {
452+
let expected = "col1,col2\nnew1,new2\n";
453+
454+
let model_dir = ModelPatch::from_example("simple")
455+
.with_file_patch(
456+
FilePatch::new("assets.csv").with_replacement(&["col1,col2", "new1,new2"]),
457+
)
458+
.build_to_tempdir()
459+
.unwrap();
460+
461+
let assets_path = model_dir.path().join("assets.csv");
462+
let assets_content = std::fs::read_to_string(assets_path).unwrap();
463+
assert_eq!(assets_content, expected);
464+
}
465+
466+
#[test]
467+
#[should_panic(
468+
expected = "Cannot set replacement content when header is set for this FilePatch"
469+
)]
470+
fn file_patch_replacement_after_header_panics() {
471+
let _ = FilePatch::new("assets.csv")
472+
.with_header("col1,col2")
473+
.with_replacement(&["col1,col2", "a,b"]);
474+
}
475+
476+
#[test]
477+
#[should_panic(
478+
expected = "Cannot set replacement content when additions/deletions are set for this FilePatch"
479+
)]
480+
fn file_patch_replacement_after_addition_panics() {
481+
let _ = FilePatch::new("assets.csv")
482+
.with_addition("a,b")
483+
.with_replacement(&["col1,col2", "a,b"]);
484+
}
485+
486+
#[test]
487+
#[should_panic(expected = "Cannot add rows when replacement content is set for this FilePatch")]
488+
fn file_patch_addition_after_replacement_panics() {
489+
let _ = FilePatch::new("assets.csv")
490+
.with_replacement(&["col1,col2", "a,b"])
491+
.with_addition("c,d");
492+
}
493+
494+
#[test]
495+
fn file_patch_with_replacement_missing_base_file_fails() {
496+
let model_patch = ModelPatch::from_example("simple").with_file_patch(
497+
FilePatch::new("not_a_real_file.csv").with_replacement(&["x,y", "1,2"]),
498+
);
499+
500+
let expected = format!(
501+
"Base file for patching does not exist: {}",
502+
std::path::PathBuf::from("examples")
503+
.join("simple")
504+
.join("not_a_real_file.csv")
505+
.display()
506+
);
507+
508+
assert_error!(model_patch.build_to_tempdir(), expected);
509+
}
510+
511+
#[test]
512+
#[should_panic(expected = "Line 1 has 2 columns but line 0 has 3")]
513+
fn file_patch_replacement_column_count_mismatch_panics() {
514+
let _ = FilePatch::new("test.csv").with_replacement(&["col1,col2,col3", "a,b"]);
515+
}
516+
382517
#[test]
383518
fn toml_patch() {
384519
// Patch to add an extra milestone year (2050)

0 commit comments

Comments
 (0)