From 1922750fa29bd0aaea111779a68810a9d44342b2 Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 10:31:43 +0200 Subject: [PATCH 1/7] Add propage_modes_error for better testing of modes --- simba/simulate.py | 2 ++ tests/test_simulate.py | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/simba/simulate.py b/simba/simulate.py index 33e78cd9..3e20ec99 100644 --- a/simba/simulate.py +++ b/simba/simulate.py @@ -129,6 +129,8 @@ def modes_simulation(schedule, scenario, args): logging.error(msg) logging.error('*'*len(msg)) logging.error(traceback.format_exc()) + if args.propagate_mode_errors: + raise e if scenario is not None and scenario.step_i > 0: # generate plot of failed scenario args.mode = args.mode[:i] + ["ABORTED"] diff --git a/tests/test_simulate.py b/tests/test_simulate.py index 796e140c..896d513f 100644 --- a/tests/test_simulate.py +++ b/tests/test_simulate.py @@ -1,3 +1,4 @@ +import traceback from argparse import Namespace import logging from pathlib import Path @@ -12,7 +13,7 @@ class TestSimulate: - + # Add propagate_mode_errors as developer setting to raise Exceptions. DEFAULT_VALUES = { "vehicle_types": example_path / "vehicle_types.json", "electrified_stations": example_path / "electrified_stations.json", @@ -39,6 +40,7 @@ class TestSimulate: "desired_soc_deps": 1, "min_charging_time": 0, "default_voltage_level": "MV", + "propagate_mode_errors": True, } def test_basic(self): @@ -48,12 +50,16 @@ def test_basic(self): def test_missing(self): # every value in DEFAULT_VALUES is expected to be set, so omitting one should raise an error values = self.DEFAULT_VALUES.copy() + # except propagate_modes_error + del self.DEFAULT_VALUES["propagate_mode_errors"] for k, v in self.DEFAULT_VALUES.items(): del values[k] with pytest.raises(Exception): simulate(Namespace(**values)) # reset values[k] = v + # restore the setting for further testing + self.DEFAULT_VALUES["propagate_mode_errors"] = values["propagate_mode_errors"] # required file missing for file_type in ["vehicle_types", "electrified_stations", "cost_parameters_file"]: @@ -100,6 +106,7 @@ def test_mode_change_charge_type(self): values["ALLOW_NEGATIVE_SOC"] = True simulate(Namespace(**values)) + def test_mode_remove_negative(self): values = self.DEFAULT_VALUES.copy() values["mode"] = "remove_negative" @@ -108,6 +115,10 @@ def test_mode_remove_negative(self): values["ALLOW_NEGATIVE_SOC"] = True simulate(Namespace(**values)) + + + + def test_mode_report(self, tmp_path): # report with cost calculation, write to tmp values = self.DEFAULT_VALUES.copy() From 33b0b434f526223fe70de2490fd71523da564d6d Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 10:56:13 +0200 Subject: [PATCH 2/7] Fix get negative rotations, and change basic_run return --- tests/test_consumption.py | 2 +- tests/test_schedule.py | 22 ++++++++++++++-------- tests/test_util.py | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/test_consumption.py b/tests/test_consumption.py index 7caca24e..5dae0408 100644 --- a/tests/test_consumption.py +++ b/tests/test_consumption.py @@ -14,7 +14,7 @@ def test_calculate_consumption(self, tmp_path): :param tmp_path: pytest fixture to create a temporary path """ - schedule, scenario = TestSchedule().basic_run() + schedule, scenario, _ = TestSchedule().basic_run() trip = next(iter(schedule.rotations.values())).trips.pop(0) consumption = trip.__class__.consumption consumption.temperatures_by_hour = {hour: hour * 2 - 15 for hour in range(0, 24)} diff --git a/tests/test_schedule.py b/tests/test_schedule.py index 09f7dc09..f98c7092 100644 --- a/tests/test_schedule.py +++ b/tests/test_schedule.py @@ -59,7 +59,7 @@ def basic_run(self): args.attach_vehicle_soc = True scen = generated_schedule.run(args) - return generated_schedule, scen + return generated_schedule, scen, args def test_mandatory_options_exit(self): """ @@ -105,7 +105,7 @@ def test_station_data_reading(self): def test_basic_run(self): """ Check if running a basic example works and if a scenario object is returned """ - schedule, scen = self.basic_run() + schedule, scen, args = self.basic_run() assert type(scen) is scenario.Scenario def test_assign_vehicles(self): @@ -131,13 +131,15 @@ def test_assign_vehicles(self): def test_calculate_consumption(self): """ Test if calling the consumption calculation works """ + # Changing self.vehicle_types can propagate to other tests + vehicle_types = deepcopy(self.vehicle_types) trip.Trip.consumption = consumption.Consumption( - self.vehicle_types, outside_temperatures=self.temperature_path, + vehicle_types, outside_temperatures=self.temperature_path, level_of_loading_over_day=self.lol_path) path_to_trips = file_root / "trips_assign_vehicles.csv" generated_schedule = schedule.Schedule.from_csv( - path_to_trips, self.vehicle_types, self.electrified_stations, **mandatory_args) + path_to_trips, vehicle_types, self.electrified_stations, **mandatory_args) # set mileage to a constant mileage = 10 @@ -173,12 +175,16 @@ def test_get_common_stations(self): def test_get_negative_rotations(self): """Check if the single rotation '1' with a negative soc is found """ - # make use of the test_run() which has to return schedule and scenario object - sched, scen = self.basic_run() - + sched, scen, args = self.basic_run() + for rot in sched.rotations.values(): + for t in rot.trips: + t.distance = 0.01 + sched.rotations["1"].trips[0].distance = 9999999 + sched.calculate_consumption() + scen = sched.run(args) neg_rots = sched.get_negative_rotations(scen) - assert '2' in neg_rots + assert ['1'] == neg_rots def test_rotation_filter(self, tmp_path): s = schedule.Schedule(self.vehicle_types, self.electrified_stations, **mandatory_args) diff --git a/tests/test_util.py b/tests/test_util.py index 07fc0b26..e140dfbf 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -49,7 +49,7 @@ def test_get_git_revision_hash(self): assert type(git_hash) is str def test_get_buffer_time(self): - schedule, scenario = TestSchedule().basic_run() + schedule, scenario, _ = TestSchedule().basic_run() trip = next(iter(schedule.rotations.values())).trips.pop(0) util.get_buffer_time(trip) buffer_time = {"10-22": 2, From f71f2359a21f39eddca03d9cdc81e807a061201f Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 11:06:29 +0200 Subject: [PATCH 3/7] Refactor basic_run --- tests/test_schedule.py | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/tests/test_schedule.py b/tests/test_schedule.py index f98c7092..e56be23d 100644 --- a/tests/test_schedule.py +++ b/tests/test_schedule.py @@ -6,6 +6,7 @@ import spice_ev.scenario as scenario from spice_ev.util import set_options_from_config +from simba.simulate import pre_simulation from tests.conftest import example_root, file_root from tests.helpers import generate_basic_schedule from simba import consumption, rotation, schedule, trip, util @@ -33,10 +34,9 @@ class TestSchedule: vehicle_types = util.uncomment_json_file(file) def basic_run(self): - """Returns a schedule and scenario after running SimBA. - :return: schedule, scenario + """Returns a schedule, scenario and args after running SimBA. + :return: schedule, scenario, args """ - path_to_trips = example_root / "trips_example.csv" # set the system variables to imitate the console call with the config argument. # first element has to be set to something or error is thrown sys.argv = ["foo", "--config", str(example_root / "simba.cfg")] @@ -44,22 +44,13 @@ def basic_run(self): args.config = example_root / "simba.cfg" args.days = None args.seed = 5 - - trip.Trip.consumption = consumption.Consumption( - self.vehicle_types, outside_temperatures=self.temperature_path, - level_of_loading_over_day=self.lol_path) - - path_to_all_station_data = example_root / "all_stations.csv" - generated_schedule = schedule.Schedule.from_csv( - path_to_trips, self.vehicle_types, self.electrified_stations, **mandatory_args, - station_data_path=path_to_all_station_data) - set_options_from_config(args, verbose=False) args.ALLOW_NEGATIVE_SOC = True args.attach_vehicle_soc = True - scen = generated_schedule.run(args) - return generated_schedule, scen, args + sched = pre_simulation(args) + scen = sched.run(args) + return sched, scen, args def test_mandatory_options_exit(self): """ @@ -105,7 +96,7 @@ def test_station_data_reading(self): def test_basic_run(self): """ Check if running a basic example works and if a scenario object is returned """ - schedule, scen, args = self.basic_run() + sched, scen, args = self.basic_run() assert type(scen) is scenario.Scenario def test_assign_vehicles(self): From b848042d1bfdcd4854470fe0726606528b824be0 Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 11:08:20 +0200 Subject: [PATCH 4/7] Make flake8 happy --- simba/simulate.py | 2 ++ tests/test_simulate.py | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/simba/simulate.py b/simba/simulate.py index 3e20ec99..8eefaee2 100644 --- a/simba/simulate.py +++ b/simba/simulate.py @@ -98,6 +98,8 @@ def modes_simulation(schedule, scenario, args): :type args: Namespace :return: final schedule and scenario :rtype: tuple + :raises Exception: If developer setting propagate_mode_errors is set and error occurs during + mode """ if type(args.mode) is not list: # backwards compatibility: run single mode diff --git a/tests/test_simulate.py b/tests/test_simulate.py index 896d513f..c510b5dc 100644 --- a/tests/test_simulate.py +++ b/tests/test_simulate.py @@ -1,4 +1,3 @@ -import traceback from argparse import Namespace import logging from pathlib import Path @@ -106,7 +105,6 @@ def test_mode_change_charge_type(self): values["ALLOW_NEGATIVE_SOC"] = True simulate(Namespace(**values)) - def test_mode_remove_negative(self): values = self.DEFAULT_VALUES.copy() values["mode"] = "remove_negative" @@ -115,10 +113,6 @@ def test_mode_remove_negative(self): values["ALLOW_NEGATIVE_SOC"] = True simulate(Namespace(**values)) - - - - def test_mode_report(self, tmp_path): # report with cost calculation, write to tmp values = self.DEFAULT_VALUES.copy() From 931bc19c5e1c05c1e1707b1e1cc94523c115ddbe Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 11:45:20 +0200 Subject: [PATCH 5/7] Add propagate_mode_errors to args parser --- simba/util.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/simba/util.py b/simba/util.py index 49ed0775..42236d4e 100644 --- a/simba/util.py +++ b/simba/util.py @@ -287,6 +287,9 @@ def get_args(): help='Remove rotations from schedule that violate assumptions. ') parser.add_argument('--show-plots', action='store_true', help='show plots for users to view in "report" mode') + parser.add_argument('--propagate_mode_errors', default=False, + help='Re-raise errors instead of continuing during simulation modes') + # #### Physical setup of environment ##### parser.add_argument('--preferred-charging-type', '-pct', default='depb', choices=['depb', 'oppb'], help="Preferred charging type. Choose one\ @@ -348,7 +351,7 @@ def get_args(): nargs=2, default=[], action='append', help='append additional argument to price signals') parser.add_argument('--optimizer_config', default=None, - help="For station_optimization a optimizer_config is needed. \ + help="For station_optimization an optimizer_config is needed. \ Input a path to an .cfg file or use the default_optimizer.cfg") parser.add_argument('--config', help='Use config file to set arguments') From eef5ecedb60e4aeb049413caf282df94b135d590 Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 11:52:07 +0200 Subject: [PATCH 6/7] Change underscores to dashes --- simba/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simba/util.py b/simba/util.py index 42236d4e..915159a6 100644 --- a/simba/util.py +++ b/simba/util.py @@ -287,7 +287,7 @@ def get_args(): help='Remove rotations from schedule that violate assumptions. ') parser.add_argument('--show-plots', action='store_true', help='show plots for users to view in "report" mode') - parser.add_argument('--propagate_mode_errors', default=False, + parser.add_argument('--propagate-mode-errors', default=False, help='Re-raise errors instead of continuing during simulation modes') # #### Physical setup of environment ##### From 6b95231b3133bb590a2c63ed655b2fcf90e94d76 Mon Sep 17 00:00:00 2001 From: "paul.scheer" Date: Thu, 10 Aug 2023 13:36:08 +0200 Subject: [PATCH 7/7] Change docstring and exception raise according to review --- simba/simulate.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/simba/simulate.py b/simba/simulate.py index 8eefaee2..e1b5a506 100644 --- a/simba/simulate.py +++ b/simba/simulate.py @@ -98,8 +98,7 @@ def modes_simulation(schedule, scenario, args): :type args: Namespace :return: final schedule and scenario :rtype: tuple - :raises Exception: If developer setting propagate_mode_errors is set and error occurs during - mode + :raises Exception: if args.propagate_mode_errors is set, re-raises error instead of continuing """ if type(args.mode) is not list: # backwards compatibility: run single mode @@ -126,13 +125,13 @@ def modes_simulation(schedule, scenario, args): schedule, scenario = func(schedule, scenario, args, i) logging.debug("Finished mode " + mode) except Exception as e: + if args.propagate_mode_errors: + raise msg = f"{e.__class__.__name__} during {mode}: {e}" logging.error('*'*len(msg)) logging.error(msg) logging.error('*'*len(msg)) logging.error(traceback.format_exc()) - if args.propagate_mode_errors: - raise e if scenario is not None and scenario.step_i > 0: # generate plot of failed scenario args.mode = args.mode[:i] + ["ABORTED"]