Skip to content

Commit

Permalink
feat: Capture conversion errors when using convert CLI.
Browse files Browse the repository at this point in the history
Alter the framework for the convert command to capture all the
conversion errors and log them to the user.
  • Loading branch information
jetuk committed Mar 12, 2024
1 parent 2133bc0 commit 1baf9ae
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 56 deletions.
71 changes: 34 additions & 37 deletions pywr-cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
mod tracing;

use crate::tracing::setup_tracing;
use anyhow::{Context, Result};
use ::tracing::info;
use anyhow::Result;
use clap::{Parser, Subcommand, ValueEnum};
#[cfg(feature = "ipm-ocl")]
use pywr_core::solvers::{ClIpmF32Solver, ClIpmF64Solver, ClIpmSolverSettings};
Expand All @@ -12,7 +13,6 @@ use pywr_core::solvers::{HighsSolver, HighsSolverSettings};
use pywr_core::solvers::{SimdIpmF64Solver, SimdIpmSolverSettings};
use pywr_core::test_utils::make_random_model;
use pywr_schema::model::{PywrModel, PywrMultiNetworkModel};
use pywr_schema::ConversionError;
use rand::SeedableRng;
use rand_chacha::ChaCha8Rng;
use std::fmt::{Display, Formatter};
Expand Down Expand Up @@ -50,16 +50,9 @@ impl Display for Solver {
#[derive(Parser)]
#[command(author, version, about, long_about = None)]
struct Cli {
// /// Optional name to operate on
// name: Option<String>,
//
// /// Sets a custom config file
// #[arg(short, long, value_name = "FILE")]
// config: Option<PathBuf>,
//
// /// Turn debugging information on
// #[arg(short, long, action = clap::ArgAction::Count)]
// debug: u8,
/// Turn debugging information on
#[arg(long, default_value_t = false)]
debug: bool,
#[command(subcommand)]
command: Option<Commands>,
}
Expand All @@ -69,6 +62,9 @@ enum Commands {
Convert {
/// Path to Pywr v1.x JSON.
model: PathBuf,
/// Stop if there is an error converting the model.
#[arg(short, long, default_value_t = false)]
stop_on_error: bool,
},

Run {
Expand All @@ -87,8 +83,6 @@ enum Commands {
/// The number of threads to use in parallel simulation.
#[arg(short, long, default_value_t = 1)]
threads: usize,
#[arg(long, default_value_t = false)]
debug: bool,
},
RunMulti {
/// Path to Pywr model JSON.
Expand All @@ -106,8 +100,6 @@ enum Commands {
/// The number of threads to use in parallel simulation.
#[arg(short, long, default_value_t = 1)]
threads: usize,
#[arg(long, default_value_t = false)]
debug: bool,
},
RunRandom {
num_systems: usize,
Expand All @@ -121,28 +113,27 @@ enum Commands {

fn main() -> Result<()> {
let cli = Cli::parse();
setup_tracing(cli.debug).unwrap();

match &cli.command {
Some(command) => match command {
Commands::Convert { model } => convert(model)?,
Commands::Convert { model, stop_on_error } => convert(model, *stop_on_error),
Commands::Run {
model,
solver,
data_path,
output_path,
parallel: _,
threads: _,
debug,
} => run(model, solver, data_path.as_deref(), output_path.as_deref(), *debug),
} => run(model, solver, data_path.as_deref(), output_path.as_deref()),
Commands::RunMulti {
model,
solver,
data_path,
output_path,
parallel: _,
threads: _,
debug,
} => run_multi(model, solver, data_path.as_deref(), output_path.as_deref(), *debug),
} => run_multi(model, solver, data_path.as_deref(), output_path.as_deref()),
Commands::RunRandom {
num_systems,
density,
Expand All @@ -156,30 +147,42 @@ fn main() -> Result<()> {
Ok(())
}

fn convert(path: &Path) -> Result<()> {
fn convert(path: &Path, stop_on_error: bool) {
if path.is_dir() {
for entry in path.read_dir().expect("read_dir call failed").flatten() {
let path = entry.path();
if path.is_file()
&& (path.extension().unwrap() == "json")
&& (!path.file_stem().unwrap().to_str().unwrap().contains("_v2"))
{
v1_to_v2(&path).with_context(|| format!("Could not convert model: `{:?}`", &path))?;
v1_to_v2(&path, stop_on_error);
}
}
} else {
v1_to_v2(path).with_context(|| format!("Could not convert model: `{:?}`", path))?;
v1_to_v2(path, stop_on_error);
}

Ok(())
}

fn v1_to_v2(path: &Path) -> std::result::Result<(), ConversionError> {
println!("Model: {}", path.display());
fn v1_to_v2(path: &Path, stop_on_error: bool) {
info!("Model: {}", path.display());

let data = std::fs::read_to_string(path).unwrap();
// Load the v1 schema
let schema: pywr_v1_schema::PywrModel = serde_json::from_str(data.as_str()).unwrap();
let schema_v2: PywrModel = schema.try_into()?;
// Convert to v2 schema and collect any errors
let (schema_v2, errors) = PywrModel::from_v1(schema);

if !errors.is_empty() {
info!("Model converted with {} errors:", errors.len());
for error in errors {
info!(" {}", error);
}
if stop_on_error {
return;
}
} else {
info!("Model converted with zero errors!");
}

// There must be a better way to do this!!
let mut new_file_name = path.file_stem().unwrap().to_os_string();
Expand All @@ -189,13 +192,9 @@ fn v1_to_v2(path: &Path) -> std::result::Result<(), ConversionError> {
let new_file_pth = path.parent().unwrap().join(new_file_name);

std::fs::write(new_file_pth, serde_json::to_string_pretty(&schema_v2).unwrap()).unwrap();

Ok(())
}

fn run(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path: Option<&Path>, debug: bool) {
setup_tracing(debug).unwrap();

fn run(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path: Option<&Path>) {
let data = std::fs::read_to_string(path).unwrap();
let data_path = data_path.or_else(|| path.parent());
let schema_v2: PywrModel = serde_json::from_str(data.as_str()).unwrap();
Expand All @@ -216,9 +215,7 @@ fn run(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path: Opti
.unwrap();
}

fn run_multi(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path: Option<&Path>, debug: bool) {
setup_tracing(debug).unwrap();

fn run_multi(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path: Option<&Path>) {
let data = std::fs::read_to_string(path).unwrap();
let data_path = data_path.or_else(|| path.parent());

Expand Down
2 changes: 0 additions & 2 deletions pywr-core/src/parameters/py.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ impl PyParameter {
}

impl Parameter<f64> for PyParameter {

fn meta(&self) -> &ParameterMeta {
&self.meta
}
Expand Down Expand Up @@ -182,7 +181,6 @@ impl Parameter<f64> for PyParameter {
}

impl Parameter<usize> for PyParameter {

fn meta(&self) -> &ParameterMeta {
&self.meta
}
Expand Down
81 changes: 64 additions & 17 deletions pywr-schema/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ pub struct Metadata {
pub minimum_version: Option<String>,
}

impl Default for Metadata {
fn default() -> Self {
Self {
title: "Untitled model".to_string(),
description: None,
minimum_version: None,
}
}
}

impl TryFrom<pywr_v1_schema::model::Metadata> for Metadata {
type Error = ConversionError;

Expand Down Expand Up @@ -64,6 +74,16 @@ pub struct Timestepper {
pub timestep: Timestep,
}

impl Default for Timestepper {
fn default() -> Self {
Self {
start: DateType::Date(NaiveDate::from_ymd_opt(2000, 1, 1).expect("Invalid date")),
end: DateType::Date(NaiveDate::from_ymd_opt(2000, 12, 31).expect("Invalid date")),
timestep: Timestep::Days(1),
}
}
}

impl TryFrom<pywr_v1_schema::model::Timestepper> for Timestepper {
type Error = ConversionError;

Expand Down Expand Up @@ -371,20 +391,38 @@ impl PywrModel {

Ok(model)
}
}

impl TryFrom<pywr_v1_schema::PywrModel> for PywrModel {
type Error = ConversionError;

fn try_from(v1: pywr_v1_schema::PywrModel) -> Result<Self, Self::Error> {
let metadata = v1.metadata.try_into()?;
let timestepper = v1.timestepper.try_into()?;
/// Convert a v1 model to a v2 model.
///
/// This function is used to convert a v1 model to a v2 model. The conversion is not always
/// possible and may result in errors. The errors are returned as a vector of [`ConversionError`]s.
/// alongside the (partially) converted model. This may result in a model that will not
/// function as expected. The user should check the errors and the converted model to ensure
/// that the conversion has been successful.
pub fn from_v1(v1: pywr_v1_schema::PywrModel) -> (Self, Vec<ConversionError>) {
let mut errors = Vec::new();

let metadata = v1.metadata.try_into().unwrap_or_else(|e| {
errors.push(e);
Metadata::default()
});

let timestepper = v1.timestepper.try_into().unwrap_or_else(|e| {
errors.push(e);
Timestepper::default()
});

let nodes = v1
.nodes
.into_iter()
.map(|n| n.try_into())
.collect::<Result<Vec<_>, _>>()?;
.filter_map(|n| match n.try_into() {
Ok(n) => Some(n),
Err(e) => {
errors.push(e);
None
}
})
.collect::<Vec<_>>();

let edges = v1.edges.into_iter().map(|e| e.into()).collect();

Expand All @@ -393,8 +431,14 @@ impl TryFrom<pywr_v1_schema::PywrModel> for PywrModel {
Some(
v1_parameters
.into_iter()
.map(|p| p.try_into_v2_parameter(None, &mut unnamed_count))
.collect::<Result<Vec<_>, _>>()?,
.filter_map(|p| match p.try_into_v2_parameter(None, &mut unnamed_count) {
Ok(p) => Some(p),
Err(e) => {
errors.push(e);
None
}
})
.collect::<Vec<_>>(),
)
} else {
None
Expand All @@ -413,12 +457,15 @@ impl TryFrom<pywr_v1_schema::PywrModel> for PywrModel {
outputs,
};

Ok(Self {
metadata,
timestepper,
scenarios: None,
network,
})
(
Self {
metadata,
timestepper,
scenarios: None,
network,
},
errors,
)
}
}

Expand Down

0 comments on commit 1baf9ae

Please sign in to comment.