EnergySystemsModellingLab / MUSE_2.0

Welcome to the MUSE 2.0 repository
GNU General Public License v3.0
1 stars 0 forks source link

Tidy settings error handling #93

Closed alexdewar closed 3 months ago

alexdewar commented 3 months ago

Description

I'm currently merging the changes from #80 into my PR and I think the error-handling could be a bit tidier. No offence @Ashmit8583! This is exactly how I did things in my example code, but I've learned a bit more about error handling in Rust since then. While errors due to reading settings files should result in a panic, I think it's cleaner to pass the errors back up the callstack a bit and let the errors be dealt with in a common place in simulation::run(). That way we can print "Something went wrong while loading settings: {thing that went wrong}" rather than having panic!s dotted about the input reading code.

I also renamed a couple of variables for consistency.

@Ashmit8583 I've tagged you as a reviewer because this is your code so it would be good if you could take a look.

Type of change

Key checklist

Further checks

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.68%. Comparing base (3b18095) to head (9817c09).

Files Patch % Lines
src/simulation.rs 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #93 +/- ## ========================================== + Coverage 71.05% 73.68% +2.63% ========================================== Files 3 3 Lines 38 38 ========================================== + Hits 27 28 +1 + Misses 11 10 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Ashmit8583 commented 3 months ago

Sure sir let me have a look at it again

On Fri, 28 Jun 2024, 2:24 pm Alex Dewar, @.***> wrote:

@.**** commented on this pull request.

In src/settings.rs https://github.com/EnergySystemsModellingLab/MUSE_2.0/pull/93#discussion_r1658739724 :

  • update_path!(settings.input_files.agents_file_path);
  • update_path!(settings.input_files.agent_objectives_file_path);
  • update_path!(settings.input_files.agent_regions_file_path);
  • update_path!(settings.input_files.assets_file_path);
  • update_path!(settings.input_files.commodities_file_path);
  • update_path!(settings.input_files.commodity_constraints_file_path);
  • update_path!(settings.input_files.commodity_costs_file_path);
  • update_path!(settings.input_files.demand_file_path);
  • update_path!(settings.input_files.demand_slicing_file_path);
  • update_path!(settings.input_files.processes_file_path);
  • update_path!(settings.input_files.process_availabilities_file_path);
  • update_path!(
  • settings
  • .input_files
  • .process_flow_share_constraints_file_path
  • );
  • update_path!(settings.input_files.process_flows_file_path);
  • update_path!(
  • settings
  • .input_files
  • .process_investment_constraints_file_path
  • );
  • update_path!(settings.input_files.process_pacs_file_path);
  • update_path!(settings.input_files.process_parameters_file_path);
  • update_path!(settings.input_files.process_regions_file_path);
  • update_path!(settings.input_files.regions_file_path);
  • update_path!(settings.input_files.time_slices_path);

It'll work if you make inputs a reference, i.e.

let inputs = &settings.input_files;

I think it's a good idea, so I'll do that.

— Reply to this email directly, view it on GitHub https://github.com/EnergySystemsModellingLab/MUSE_2.0/pull/93#discussion_r1658739724, or unsubscribe https://github.com/notifications/unsubscribe-auth/BAWEAH7TFOEFQZ37SFY45C3ZJVPZJAVCNFSM6AAAAABKBTX5FGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNBYGAYDOMRYGE . You are receiving this because you were mentioned.Message ID: @.***>