73 Selecting specific Input folder (and/or Euromod Policy Schedule) per-run via config file#207
Conversation
…/73-selecting-specific-input-folder
There was a problem hiding this comment.
Pull Request Overview
This PR enables per-run configuration of input, initial population, and Euromod policy schedule directories via a config file, allowing multiple scenarios with separate data folders.
- Introduces
parameter_argsfor directory settings in YAML and updates path constructions to useParameters.getInputDirectory()and similar methods. - Adds reflection-based
setExperimentFoldersto configure MicrosimExperimentinput/output paths. - Updates tests and CSV/database parsers to use the new directory parameters.
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
PersonTest.java |
Sets custom input directory for EQ-5D parameter tests |
TaxDonorDataParser.java |
Uses Parameters.getInputDirectory() in JDBC and Hibernate URL |
SimPathsStart.java |
Replaces hard-coded “input” paths with parameters and updates exception message |
SimPathsMultiRun.java |
Adds handling of input_directory and working_directory parameters, and reflection helpers |
DataParser.java |
Uses Parameters.getInputDirectory() for H2 connection |
config/default.yml |
Adds parameter_args.INPUT_DIRECTORY entry |
Comments suppressed due to low confidence (1)
config/default.yml:65
- YAML key
INPUT_DIRECTORYis uppercase but the code expects"input_directory". Rename the key to match the switch-case or adjust the code to accept the uppercase variant.
INPUT_DIRECTORY: input
| @BeforeEach | ||
| public void setupLawrenceCoefficients() { | ||
|
|
||
| Parameters.setInputDirectory("src/test/java/simpaths/testinput"); |
There was a problem hiding this comment.
[nitpick] Modifying a global Parameters directory in @beforeeach without resetting it may lead to state leakage between tests; consider using JUnit's @tempdir or resetting to a default value in an @AfterEach.
| try { | ||
| Class.forName("org.h2.Driver"); | ||
| conn = DriverManager.getConnection("jdbc:h2:file:./input" + File.separator + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); | ||
| conn = DriverManager.getConnection("jdbc:h2:file:" + Parameters.getInputDirectory() + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); |
There was a problem hiding this comment.
Missing file separator between getInputDirectory() and input; this may produce an invalid path. Use Parameters.getInputDirectory() + File.separator + "input".
| conn = DriverManager.getConnection("jdbc:h2:file:" + Parameters.getInputDirectory() + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); | |
| conn = DriverManager.getConnection("jdbc:h2:file:" + Parameters.getInputDirectory() + File.separator + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); |
| EntityManager em = Persistence.createEntityManagerFactory("tax-database").createEntityManager(); | ||
| // access database and obtain donor pool | ||
| Map propertyMap = new HashMap(); | ||
| propertyMap.put("hibernate.connection.url", "jdbc:h2:file:" + Parameters.getInputDirectory() + "input" + ";TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE"); |
There was a problem hiding this comment.
Path concatenation omits a separator before input, leading to an incorrect JDBC URL. Insert File.separator between the directory and "input".
| propertyMap.put("hibernate.connection.url", "jdbc:h2:file:" + Parameters.getInputDirectory() + "input" + ";TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE"); | |
| propertyMap.put("hibernate.connection.url", "jdbc:h2:file:" + Parameters.getInputDirectory() + File.separator + "input" + ";TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE"); |
|
|
||
| //Adjust the country and year to the value read from Excel, which is updated when the database is rebuilt. Otherwise it will set the country and year to the last one used to build the database | ||
| MultiKeyCoefficientMap lastDatabaseCountryAndYear = ExcelAssistant.loadCoefficientMap("input" + File.separator + Parameters.DatabaseCountryYearFilename + ".xlsx", "Data", 1, 1); | ||
| MultiKeyCoefficientMap lastDatabaseCountryAndYear = ExcelAssistant.loadCoefficientMap(Parameters.getInputDirectory() + Parameters.DatabaseCountryYearFilename + ".xlsx", "Data", 1, 1); |
There was a problem hiding this comment.
Concatenating getInputDirectory() and filename without a separator may cause missing-path errors; consider adding File.separator between them.
| MultiKeyCoefficientMap lastDatabaseCountryAndYear = ExcelAssistant.loadCoefficientMap(Parameters.getInputDirectory() + Parameters.DatabaseCountryYearFilename + ".xlsx", "Data", 1, 1); | |
| MultiKeyCoefficientMap lastDatabaseCountryAndYear = ExcelAssistant.loadCoefficientMap( | |
| java.nio.file.Paths.get(Parameters.getInputDirectory(), Parameters.DatabaseCountryYearFilename + ".xlsx").toString(), | |
| "Data", 1, 1); |
| throw new FileNotFoundException("Policy Schedule file '"+ File.separator + Parameters.getInputDirectory() + | ||
| Parameters.EUROMODpolicyScheduleFilename + ".xlsx` doesn't exist. " + | ||
| "Provide excel file or use `--rewrite-policy-schedule` to re-construct from available policy files."); |
There was a problem hiding this comment.
The exception message is confusing: it prefixes the path with File.separator and includes a stray backtick (). Refactor to show the full path correctly, e.g., "Policy Schedule file '" + fullPath + "' doesn't exist."`.
| throw new FileNotFoundException("Policy Schedule file '"+ File.separator + Parameters.getInputDirectory() + | |
| Parameters.EUROMODpolicyScheduleFilename + ".xlsx` doesn't exist. " + | |
| "Provide excel file or use `--rewrite-policy-schedule` to re-construct from available policy files."); | |
| throw new FileNotFoundException("Policy Schedule file '" + Parameters.getInputDirectory() + | |
| Parameters.EUROMODpolicyScheduleFilename + ".xlsx' doesn't exist. " + | |
| "Provide the Excel file or use `--rewrite-policy-schedule` to re-construct it from available policy files."); |
|
|
||
| // load previously stored values for policy description and initiation year | ||
| MultiKeyCoefficientMap previousEUROMODfileInfo = ExcelAssistant.loadCoefficientMap("input" + File.separator + Parameters.EUROMODpolicyScheduleFilename + ".xlsx", country.toString(), 1, 3); | ||
| MultiKeyCoefficientMap previousEUROMODfileInfo = ExcelAssistant.loadCoefficientMap(Parameters.getInputDirectory() + Parameters.EUROMODpolicyScheduleFilename + ".xlsx", country.toString(), 1, 3); |
There was a problem hiding this comment.
Missing path separator before the filename; add File.separator between getInputDirectory() and the schedule filename.
| MultiKeyCoefficientMap previousEUROMODfileInfo = ExcelAssistant.loadCoefficientMap(Parameters.getInputDirectory() + Parameters.EUROMODpolicyScheduleFilename + ".xlsx", country.toString(), 1, 3); | |
| MultiKeyCoefficientMap previousEUROMODfileInfo = ExcelAssistant.loadCoefficientMap(Parameters.getInputDirectory() + File.separator + Parameters.EUROMODpolicyScheduleFilename + ".xlsx", country.toString(), 1, 3); |
| try { | ||
| Field inputDir = Experiment.class.getDeclaredField("inputFolder"); | ||
| inputDir.setAccessible(true); | ||
| inputDir.set(Experiment.class, Parameters.getInputDirectory()); |
There was a problem hiding this comment.
For setting a static field via reflection, the first argument to Field.set should be null, not Experiment.class. Use inputDir.set(null, Parameters.getInputDirectory()).
| inputDir.set(Experiment.class, Parameters.getInputDirectory()); | |
| inputDir.set(null, Parameters.getInputDirectory()); |
| SimPathsModel model = new SimPathsModel(Country.getCountryFromNameString(countryString), startYear); | ||
| if (persist_population) model.setPersistPopulation(true); | ||
| if (persist_root) model.setPersistDatabasePath("./input/input"); | ||
| if (persist_root) model.setPersistDatabasePath(Parameters.getInputDirectory() + "input"); |
There was a problem hiding this comment.
Concatenation lacks a separator before input; include File.separator (or ensure the directory string ends with one) to avoid malformed paths.
| if (persist_root) model.setPersistDatabasePath(Parameters.getInputDirectory() + "input"); | |
| if (persist_root) model.setPersistDatabasePath(ensureTrailingSeparator(Parameters.getInputDirectory()) + "input"); |
| try { | ||
| Class.forName("org.h2.Driver"); | ||
| conn = DriverManager.getConnection("jdbc:h2:file:./input" + File.separator + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); | ||
| conn = DriverManager.getConnection("jdbc:h2:file:" + Parameters.getInputDirectory() + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); |
There was a problem hiding this comment.
Missing file separator when building the H2 JDBC URL; add File.separator between the configured directory and input.
| conn = DriverManager.getConnection("jdbc:h2:file:" + Parameters.getInputDirectory() + "input;TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); | |
| conn = DriverManager.getConnection("jdbc:h2:file:" + java.nio.file.Paths.get(Parameters.getInputDirectory(), "input").toString() + ";TRACE_LEVEL_FILE=0;TRACE_LEVEL_SYSTEM_OUT=0;AUTO_SERVER=TRUE", "sa", ""); |
|
@justin-ven @pbronka - this is mostly a structural change, would allow parameters to point runs to different places to get input parts (can even set working directory outside of directory with code in it). tested manually with a folder called "impoot" and it successfully ran! |
|
Looks OK to me Andy - we can always debug later if necessary |
What
Why