From f4038240432a28c1eed48c835bdfb8c54791c395 Mon Sep 17 00:00:00 2001 From: rakow Date: Tue, 13 Feb 2024 18:11:52 +0100 Subject: [PATCH] Application contrib maintenance (#3108) * add second run method for convenience * improve config update from yaml and made it public * implement methods to run automatically from gui * fixes for run from gui * move command classes to enable usage of config path * reorder if statements * fix merging of default arguments * support list of doubles and ints as parameters * fix merge config if there is only one param set * add heuristics to detect if run from desktop on windows * fix detection of powershell * add checks if run from intelij --- .../matsim/application/ApplicationUtils.java | 207 +++++++++++++++++- .../matsim/application/MATSimApplication.java | 156 ++++++------- .../{commands => }/RunScenario.java | 5 +- .../application/{commands => }/ShowGUI.java | 18 +- .../prepare/network/CleanNetwork.java | 6 +- .../application/ConfigYamlUpdateTest.java | 195 +++++++++++++++++ .../ConfigYamlUpdateTest/multiLevel.yml | 11 + .../ConfigYamlUpdateTest/standard.yml | 14 ++ .../core/config/ReflectiveConfigGroup.java | 43 +++- .../java/org/matsim/run/gui/ExeRunner.java | 7 + .../config/ReflectiveConfigGroupTest.java | 9 +- 11 files changed, 557 insertions(+), 114 deletions(-) rename contribs/application/src/main/java/org/matsim/application/{commands => }/RunScenario.java (67%) rename contribs/application/src/main/java/org/matsim/application/{commands => }/ShowGUI.java (65%) create mode 100644 contribs/application/src/test/java/org/matsim/application/ConfigYamlUpdateTest.java create mode 100644 contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/multiLevel.yml create mode 100644 contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/standard.yml diff --git a/contribs/application/src/main/java/org/matsim/application/ApplicationUtils.java b/contribs/application/src/main/java/org/matsim/application/ApplicationUtils.java index bc9c97f08b3..24b3d712bb8 100644 --- a/contribs/application/src/main/java/org/matsim/application/ApplicationUtils.java +++ b/contribs/application/src/main/java/org/matsim/application/ApplicationUtils.java @@ -1,5 +1,9 @@ package org.matsim.application; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.matsim.api.core.v01.Scenario; @@ -7,11 +11,14 @@ import org.matsim.application.options.InputOptions; import org.matsim.application.options.OutputOptions; import org.matsim.core.config.Config; +import org.matsim.core.config.ConfigAliases; +import org.matsim.core.config.ConfigGroup; import org.matsim.core.config.ConfigUtils; import org.matsim.core.scenario.ScenarioUtils; import org.matsim.core.utils.io.IOUtils; import picocli.CommandLine; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; @@ -21,13 +28,20 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.PathMatcher; -import java.util.Optional; +import java.util.*; +import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.stream.StreamSupport; public class ApplicationUtils { private static final Logger log = LogManager.getLogger(ApplicationUtils.class); + /** + * This encoding indicates command line was used to start the jar. + */ + private static final String WIN_CLI_ENCODING = "cp850"; + private ApplicationUtils() { } @@ -46,7 +60,187 @@ public static String[] mergeArgs(String[] args, String... defaultArgs) { } /** - * Extends a context (usually config location) with an relative filename. + * Utility method to check if a jar might be run from the desktop (using double-click). + */ + public static boolean isRunFromDesktop() { + + // check if gui was explicitly enabled + String env = System.getenv().getOrDefault("RUN_GUI", "undefined"); + if (env.equalsIgnoreCase("true") || env.equals("1")) + return true; + else if (env.equalsIgnoreCase("false") || env.equals("0")) + return false; + + String property = System.getProperty("RUN_GUI", "undefined"); + if (property.equalsIgnoreCase("true") || property.equals("1")) + return true; + else if (property.equalsIgnoreCase("false") || property.equals("0")) + return false; + + String macIdentifier = System.getenv().getOrDefault("__CFBundleIdentifier", "none"); + + if (macIdentifier.equals("com.apple.java.JarLauncher") || macIdentifier.equals("com.apple.JavaLauncher")) + return true; + + String os = System.getProperty("os.name"); + + if (os.toLowerCase().startsWith("windows")) { + + // presence of the prompt variable indicates that the jar was run from the command line + boolean hasPrompt = System.getenv().containsKey("PROMPT"); + + // this prompt is not set in PowerShell, so we need another check + if (hasPrompt) + return false; + + // stdout.encoding from CLI are usually cp850 + String encoding = System.getProperty("stdout.encoding", "none"); + String sunEncoding = System.getProperty("sun.stdout.encoding", "none"); + + if (encoding.equals(WIN_CLI_ENCODING) || sunEncoding.equals(WIN_CLI_ENCODING)) + return false; + + // Run from intelij, will not start the gui by default + if (System.getenv().containsKey("IDEA_INITIAL_DIRECTORY")) + return false; + // also file.encoding=UTF-8, seems to be set by default in IntelliJ + + // if no other cli indicators are present, we have to assume that the jar was run from the desktop + return true; + } + + return false; + + } + + /** + * Apply run configuration in yaml format. + */ + public static void applyConfigUpdate(Config config, Path yaml) { + + if (!Files.exists(yaml)) { + throw new IllegalArgumentException("Desired run config does not exist:" + yaml); + } + + ObjectMapper mapper = new ObjectMapper(new YAMLFactory() + .enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)); + + ConfigAliases aliases = new ConfigAliases(); + Deque emptyStack = new ArrayDeque<>(); + + try (BufferedReader reader = Files.newBufferedReader(yaml)) { + + JsonNode node = mapper.readTree(reader); + + Iterator> fields = node.fields(); + + while (fields.hasNext()) { + Map.Entry field = fields.next(); + String configGroupName = aliases.resolveAlias(field.getKey(), emptyStack); + ConfigGroup group = config.getModules().get(configGroupName); + if (group == null) { + log.warn("Config group not found: {}", configGroupName); + continue; + } + + applyNodeToConfigGroup(field.getValue(), group); + } + + } catch (IOException e) { + throw new UncheckedIOException(e); + } + + } + + /** + * Sets the json config into + */ + private static void applyNodeToConfigGroup(JsonNode node, ConfigGroup group) { + + Iterator> fields = node.fields(); + + while (fields.hasNext()) { + Map.Entry field = fields.next(); + + if (isParameterSet(field.getValue())) { + + // store the current parameters sets, newly added sets are not merged with each other + List params = new ArrayList<>(group.getParameterSets(field.getKey())); + + for (JsonNode item : field.getValue()) { + applyNodeAsParameterSet(field.getKey(), item, group, params); + } + } else { + + if (field.getValue().isTextual()) + group.addParam(field.getKey(), field.getValue().textValue()); + else if (field.getValue().isArray()) { + // arrays are joined using "," + Stream stream = StreamSupport.stream(field.getValue().spliterator(), false); + String string = stream.map(n -> n.isTextual() ? n.textValue() : n.toString()).collect(Collectors.joining(",")); + group.addParam(field.getKey(), string); + } else + group.addParam(field.getKey(), field.getValue().toString()); + } + } + } + + /** + * Any array of complex object can be considered a config group. + */ + private static boolean isParameterSet(JsonNode node) { + + if (!node.isArray()) + return false; + + // any object can be assigned as parameter set + for (JsonNode el : node) { + if (!el.isObject()) + return false; + } + + return true; + } + + /** + * Handle possible update and creation of parameter sets within a config group. + */ + private static void applyNodeAsParameterSet(String groupName, JsonNode item, ConfigGroup group, List params) { + + Iterator> it = item.fields(); + + // There was at least one matching group + boolean matched = false; + + while (!params.isEmpty() && it.hasNext()) { + + Map.Entry attr = it.next(); + List candidates = params.stream() + .filter(p -> p.getParams().containsKey(attr.getKey())) + .filter(p -> p.getParams().get(attr.getKey()) + .equals(attr.getValue().isTextual() ? attr.getValue().textValue() : attr.getValue().toString())) + .toList(); + + if (candidates.isEmpty()) + break; + + matched = true; + params = candidates; + } + + if (params.size() > 1) { + throw new IllegalArgumentException("Ambiguous parameter set: " + item); + } else if (params.size() == 1 && matched) { + applyNodeToConfigGroup(item, params.get(0)); + } else { + ConfigGroup newGroup = group.createParameterSet(groupName); + group.addParameterSet(newGroup); + applyNodeToConfigGroup(item, newGroup); + } + } + + /** + * Extends a context (usually config location) with a relative filename. * If the results is a local file, the path will be returned. Otherwise, it will be an url. * The results can be used as input for command line parameter or {@link IOUtils#resolveFileOrResource(String)}. * @@ -75,10 +269,11 @@ public static Path globFile(Path path, String pattern) { PathMatcher m = path.getFileSystem().getPathMatcher("glob:" + pattern); try { - return Files.list(path) - .filter(p -> m.matches(p.getFileName())) - .findFirst() - .orElseThrow(() -> new IllegalStateException("No " + pattern + " file found.")); + try (Stream list = Files.list(path)) { + return list.filter(p -> m.matches(p.getFileName())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("No " + pattern + " file found.")); + } } catch (IOException e) { throw new RuntimeException(e); } diff --git a/contribs/application/src/main/java/org/matsim/application/MATSimApplication.java b/contribs/application/src/main/java/org/matsim/application/MATSimApplication.java index c1951c1b2a4..ecda5e0200a 100644 --- a/contribs/application/src/main/java/org/matsim/application/MATSimApplication.java +++ b/contribs/application/src/main/java/org/matsim/application/MATSimApplication.java @@ -1,17 +1,10 @@ package org.matsim.application; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; -import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; import com.google.common.collect.Lists; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.matsim.api.core.v01.Scenario; -import org.matsim.application.commands.RunScenario; -import org.matsim.application.commands.ShowGUI; import org.matsim.core.config.Config; -import org.matsim.core.config.ConfigAliases; import org.matsim.core.config.ConfigGroup; import org.matsim.core.config.ConfigUtils; import org.matsim.core.config.groups.ControllerConfigGroup; @@ -23,15 +16,12 @@ import picocli.CommandLine; import javax.annotation.Nullable; -import java.io.BufferedReader; import java.io.File; -import java.io.IOException; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Field; -import java.nio.file.Files; import java.nio.file.Path; import java.util.*; import java.util.concurrent.Callable; @@ -78,6 +68,8 @@ public abstract class MATSimApplication implements Callable, CommandLin private static final Logger log = LogManager.getLogger(MATSimApplication.class); + private static final String ARGS_DELIMITER = "ยง$"; + public static final String COLOR = "@|bold,fg(81) "; static final String DEFAULT_NAME = "MATSimApplication"; static final String HEADER = COLOR + @@ -170,7 +162,7 @@ public Integer call() throws Exception { Objects.requireNonNull(config); if (specs != null) - applySpecs(config, specs); + ApplicationUtils.applyConfigUpdate(config, specs); if (remainingArgs != null) { String[] args = remainingArgs.stream().map(s -> s.replace("-c:", "--config:")).toArray(String[]::new); @@ -210,99 +202,22 @@ public Integer call() throws Exception { } catch (Exception e) { log.error("Error running post-processing", e); } - } - } return 0; } - /** - * Apply given specs to config. - */ - private static void applySpecs(Config config, Path specs) { - - if (!Files.exists(specs)) { - throw new IllegalArgumentException("Desired run config does not exist:" + specs); - } - - ObjectMapper mapper = new ObjectMapper(new YAMLFactory() - .enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)); - - ConfigAliases aliases = new ConfigAliases(); - Deque emptyStack = new ArrayDeque<>(); - - try (BufferedReader reader = Files.newBufferedReader(specs)) { - - JsonNode node = mapper.readTree(reader); - - Iterator> fields = node.fields(); - - while (fields.hasNext()) { - Map.Entry field = fields.next(); - String configGroupName = aliases.resolveAlias(field.getKey(), emptyStack); - ConfigGroup group = config.getModules().get(configGroupName); - if (group == null) { - log.warn("Config group not found: {}", configGroupName); - continue; - } - - applyNodeToConfigGroup(field.getValue(), group); - } - - } catch (IOException e) { - e.printStackTrace(); - } - + File getConfigPath() { + return configPath; } - /** - * Sets the json config into - */ - private static void applyNodeToConfigGroup(JsonNode node, ConfigGroup group) { - - Iterator> fields = node.fields(); - - while (fields.hasNext()) { - Map.Entry field = fields.next(); - - if (field.getValue().isArray()) { - - Collection params = group.getParameterSets(field.getKey()); - - // single node and entry merged directly - if (field.getValue().size() == 1 && params.size() == 1) { - applyNodeToConfigGroup(field.getValue().get(0), params.iterator().next()); - } else { - - for (JsonNode item : field.getValue()) { - - Map.Entry first = item.fields().next(); - Optional m = params.stream().filter(p -> p.getParams().get(first.getKey()).equals(first.getValue().textValue())).findFirst(); - if (m.isEmpty()) - throw new IllegalArgumentException("Could not find matching param by key" + first); - - applyNodeToConfigGroup(item, m.get()); - } - } - - } else { - - if (!field.getValue().isValueNode()) - throw new IllegalArgumentException("Received complex value type instead of primitive: " + field.getValue()); - - - if (field.getValue().isTextual()) - group.addParam(field.getKey(), field.getValue().textValue()); - else - group.addParam(field.getKey(), field.getValue().toString()); - } - } + @Nullable + String getDefaultScenario() { + return defaultScenario; } - /** * Custom module configs that will be added to the {@link Config} object. * @@ -429,6 +344,13 @@ public static void run(Class clazz, String... args) l.addAll(Arrays.asList(args)); args = l.toArray(new String[0]); + + // Pass stored args to the instance as well + if (System.getenv().containsKey("MATSIM_GUI_ARGS")) { + String[] guiArgs = System.getenv("MATSIM_GUI_ARGS").split(ARGS_DELIMITER); + if (guiArgs.length > 0) + args = ApplicationUtils.mergeArgs(args, guiArgs); + } } prepareArgs(args); @@ -451,6 +373,50 @@ public static void run(Class clazz, String... args) System.exit(code); } + /** + * Convenience method to run a scenario from code or automatically with gui when desktop application is detected. + * This method may also be used to predefine some default arguments. + * @param clazz class of the scenario to run + * @param args pass arguments from the main method + * @param defaultArgs predefined default arguments that will always be present + */ + public static void runWithDefaults(Class clazz, String[] args, String... defaultArgs) { + + if (ApplicationUtils.isRunFromDesktop() && args.length == 0) { + + if (defaultArgs.length > 0) { + String value = String.join(ARGS_DELIMITER, defaultArgs); + System.setProperty("MATSIM_GUI_ARGS", value); + } + + // args are empty when run from desktop and is not used + run(clazz, "gui"); + + } else { + // run if no other command is present + if (args.length > 0) { + // valid command is present + if (args[0].equals("run") || args[0].equals("prepare") || args[0].equals("analysis") || args[0].equals("gui") ){ + // If this is a run command, the default args can be applied + if (args[0].equals("run")) + args = ApplicationUtils.mergeArgs(args, defaultArgs); + + } else { + // Automatically add run command + String[] runArgs = ApplicationUtils.mergeArgs(new String[]{"run"}, defaultArgs); + args = ApplicationUtils.mergeArgs(defaultArgs, runArgs); + } + + } else + // Automatically add run command + args = ApplicationUtils.mergeArgs(new String[]{"run"}, defaultArgs); + + log.info("Running {} with: {}", clazz.getSimpleName(), String.join(" ", args)); + + run(clazz, args); + } + } + /** * Calls an application class and forwards any exceptions. * @@ -512,7 +478,7 @@ public static Controler prepare(MATSimApplication app, Config config, String... config = tmp != null ? tmp : config; if (app.specs != null) { - applySpecs(config, app.specs); + ApplicationUtils.applyConfigUpdate(config, app.specs); } if (app.remainingArgs != null) { @@ -744,7 +710,7 @@ public Integer call() throws Exception { } /** - * Option to switch post processing behavour + * Option to switch post-processing behaviour */ public enum PostProcessOption { diff --git a/contribs/application/src/main/java/org/matsim/application/commands/RunScenario.java b/contribs/application/src/main/java/org/matsim/application/RunScenario.java similarity index 67% rename from contribs/application/src/main/java/org/matsim/application/commands/RunScenario.java rename to contribs/application/src/main/java/org/matsim/application/RunScenario.java index 3221069de95..460d5585ee5 100644 --- a/contribs/application/src/main/java/org/matsim/application/commands/RunScenario.java +++ b/contribs/application/src/main/java/org/matsim/application/RunScenario.java @@ -1,12 +1,11 @@ -package org.matsim.application.commands; +package org.matsim.application; -import org.matsim.application.MATSimApplication; import picocli.CommandLine; import java.util.concurrent.Callable; @CommandLine.Command(name = "run", description = "Run the scenario") -public class RunScenario implements Callable { +class RunScenario implements Callable { @CommandLine.ParentCommand private MATSimApplication app; diff --git a/contribs/application/src/main/java/org/matsim/application/commands/ShowGUI.java b/contribs/application/src/main/java/org/matsim/application/ShowGUI.java similarity index 65% rename from contribs/application/src/main/java/org/matsim/application/commands/ShowGUI.java rename to contribs/application/src/main/java/org/matsim/application/ShowGUI.java index 443f6a5cccb..e394bfd7b79 100644 --- a/contribs/application/src/main/java/org/matsim/application/commands/ShowGUI.java +++ b/contribs/application/src/main/java/org/matsim/application/ShowGUI.java @@ -1,14 +1,14 @@ -package org.matsim.application.commands; +package org.matsim.application; -import org.matsim.application.MATSimApplication; import org.matsim.run.gui.Gui; import picocli.CommandLine; +import java.io.File; import java.util.concurrent.Callable; import java.util.concurrent.Future; @CommandLine.Command(name = "gui", description = "Run the scenario through the MATSim GUI") -public class ShowGUI implements Callable { +class ShowGUI implements Callable { @CommandLine.ParentCommand private MATSimApplication parent; @@ -26,7 +26,17 @@ public Integer call() throws Exception { name = name.substring(MATSimApplication.COLOR.length(), name.length() - 4); } - Future f = Gui.show(name, parent.getClass()); + File configFile = null; + + // Try to load default config file + if (parent.getDefaultScenario() != null && new File(parent.getDefaultScenario()).exists()) + configFile = new File(parent.getDefaultScenario()); + + // override the default if present + if (parent.getConfigPath() != null && parent.getConfigPath().exists()) + configFile = parent.getConfigPath(); + + Future f = Gui.show(name, parent.getClass(), configFile); Gui gui = f.get(); diff --git a/contribs/application/src/main/java/org/matsim/application/prepare/network/CleanNetwork.java b/contribs/application/src/main/java/org/matsim/application/prepare/network/CleanNetwork.java index 878cfad8156..8b3e621ac22 100644 --- a/contribs/application/src/main/java/org/matsim/application/prepare/network/CleanNetwork.java +++ b/contribs/application/src/main/java/org/matsim/application/prepare/network/CleanNetwork.java @@ -11,8 +11,8 @@ import java.util.Set; @CommandLine.Command( - name = "clean-network", - description = "Ensures that all links in the network are strongly connected." + name = "clean-network", + description = "Ensures that all links in the network are strongly connected." ) public class CleanNetwork implements MATSimAppCommand { @@ -22,7 +22,7 @@ public class CleanNetwork implements MATSimAppCommand { @CommandLine.Option(names = "--output", description = "Output path", required = true) private Path output; - @CommandLine.Option(names = "--modes", description = "List of modes to clean", defaultValue = TransportMode.car) + @CommandLine.Option(names = "--modes", description = "List of modes to clean", split = ",", defaultValue = TransportMode.car) private Set modes; @Override diff --git a/contribs/application/src/test/java/org/matsim/application/ConfigYamlUpdateTest.java b/contribs/application/src/test/java/org/matsim/application/ConfigYamlUpdateTest.java new file mode 100644 index 00000000000..0c90c69c854 --- /dev/null +++ b/contribs/application/src/test/java/org/matsim/application/ConfigYamlUpdateTest.java @@ -0,0 +1,195 @@ +package org.matsim.application; + + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.matsim.core.config.Config; +import org.matsim.core.config.ConfigGroup; +import org.matsim.core.config.ConfigUtils; +import org.matsim.core.config.ReflectiveConfigGroup; +import org.matsim.testcases.MatsimTestUtils; + +import java.nio.file.Path; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +public class ConfigYamlUpdateTest { + + @RegisterExtension + private MatsimTestUtils utils = new MatsimTestUtils(); + + @Test + void standard() { + + Config config = ConfigUtils.createConfig(); + Path input = Path.of(utils.getClassInputDirectory()); + + ApplicationUtils.applyConfigUpdate( + config, input.resolve("standard.yml") + ); + + assertThat(config.controller().getRunId()).isEqualTo("567"); + assertThat(config.global().getNumberOfThreads()).isEqualTo(8); + + assertThat(config.scoring().getOrCreateModeParams("car").getConstant()).isEqualTo(-1); + assertThat(config.scoring().getOrCreateModeParams("bike").getConstant()).isEqualTo(-2); + } + + @Test + void createParamSet() { + + Config config = ConfigUtils.createConfig(); + Path input = Path.of(utils.getClassInputDirectory()); + + TestConfigGroup testGroup = ConfigUtils.addOrGetModule(config, TestConfigGroup.class); + + ApplicationUtils.applyConfigUpdate( + config, input.resolve("multiLevel.yml") + ); + + testGroup.addParam("values", "1, 2, 3"); + + assertThat(testGroup.values) + .containsExactly(1, 2, 3); + + Collection params = testGroup.getParameterSets("params"); + + assertThat(params).hasSize(2); + + Iterator it = params.iterator(); + TestParamSet next = (TestParamSet) it.next(); + + assertThat(next.getParams().get("mode")).isEqualTo("car"); + assertThat(next.getParams().get("values")).isEqualTo("-1.0, -2.0"); + assertThat(next.values).containsExactly(-1d, -2d); + + next = (TestParamSet) it.next(); + + assertThat(next.getParams().get("mode")).isEqualTo("bike"); + assertThat(next.getParams().get("values")).isEqualTo("3.0, 4.0"); + assertThat(next.getParams().get("extra")).isEqualTo("extra"); + } + + + @Test + void multiLevel() { + + Config config = ConfigUtils.createConfig(); + Path input = Path.of(utils.getClassInputDirectory()); + + TestConfigGroup testGroup = ConfigUtils.addOrGetModule(config, TestConfigGroup.class); + + testGroup.addParameterSet(new TestParamSet("car", "person", "work")); + testGroup.addParameterSet(new TestParamSet("bike", "person", "work")); + + ApplicationUtils.applyConfigUpdate( + config, input.resolve("multiLevel.yml") + ); + + Collection params = testGroup.getParameterSets("params"); + assertThat(params).hasSize(2); + + Iterator it = params.iterator(); + ConfigGroup next = it.next(); + + // These parameters are recognized as lists correctly + assertThat(next.getParams().get("values")).isEqualTo("-1.0, -2.0"); + + next = it.next(); + assertThat(next.getParams().get("values")).isEqualTo("3.0, 4.0"); + assertThat(next.getParams().get("extra")).isEqualTo("extra"); + + } + + @Test + void updateOne() { + + Config config = ConfigUtils.createConfig(); + Path input = Path.of(utils.getClassInputDirectory()); + + TestConfigGroup testGroup = ConfigUtils.addOrGetModule(config, TestConfigGroup.class); + + testGroup.addParameterSet(new TestParamSet("car", "person", "work")); + + ApplicationUtils.applyConfigUpdate( + config, input.resolve("multiLevel.yml") + ); + + Collection params = testGroup.getParameterSets("params"); + assertThat(params).hasSize(2); + + Iterator it = params.iterator(); + ConfigGroup next = it.next(); + + assertThat(next.getParams().get("mode")).isEqualTo("car"); + assertThat(next.getParams().get("values")).isEqualTo("-1.0, -2.0"); + + next = it.next(); + assertThat(next.getParams().get("mode")).isEqualTo("bike"); + assertThat(next.getParams().get("values")).isEqualTo("3.0, 4.0"); + assertThat(next.getParams().get("extra")).isEqualTo("extra"); + } + + @Test + void ambiguous() { + + Config config = ConfigUtils.createConfig(); + Path input = Path.of(utils.getClassInputDirectory()); + + TestConfigGroup testGroup = ConfigUtils.addOrGetModule(config, TestConfigGroup.class); + + testGroup.addParameterSet(new TestParamSet("car", "person", "work")); + testGroup.addParameterSet(new TestParamSet("car", "person", "home")); + + // This should fail because the parameter set is ambiguous + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> { + ApplicationUtils.applyConfigUpdate( + config, input.resolve("multiLevel.yml") + ); + }); + } + + + public static final class TestConfigGroup extends ReflectiveConfigGroup { + + @Parameter + private List values; + + public TestConfigGroup() { + super("test"); + } + + @Override + public ConfigGroup createParameterSet(String type) { + if (type.equals("params")) { + return new TestParamSet(); + } + + return super.createParameterSet(type); + } + } + + + public static final class TestParamSet extends ReflectiveConfigGroup { + + @Parameter + private List values; + + public TestParamSet() { + super("params", true); + } + + public TestParamSet(String mode, String subpopulation, String activity) { + super("params", true); + + this.addParam("mode", mode); + this.addParam("subpopulation", subpopulation); + this.addParam("activity", activity); + } + } + +} diff --git a/contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/multiLevel.yml b/contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/multiLevel.yml new file mode 100644 index 00000000000..f1d3c105534 --- /dev/null +++ b/contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/multiLevel.yml @@ -0,0 +1,11 @@ + +test: + values: [1,2,3] + params: + - mode: car + subpopulation: person + values: ["-1", "-2"] + - mode: bike + subpopulation: person + values: [3, 4] + extra: "extra" \ No newline at end of file diff --git a/contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/standard.yml b/contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/standard.yml new file mode 100644 index 00000000000..dc25f14bdf3 --- /dev/null +++ b/contribs/application/test/input/org/matsim/application/ConfigYamlUpdateTest/standard.yml @@ -0,0 +1,14 @@ +controler: + runId: 567 + +global: + numberOfThreads: 8 + +planCalcScore: + + scoringParameters: + - modeParams: + - mode: car + constant: -1 + - mode: bike + constant: -2 \ No newline at end of file diff --git a/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java b/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java index 3a00441d761..55da3166090 100644 --- a/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java +++ b/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java @@ -256,7 +256,10 @@ private static boolean checkType(Type type) { var rawType = pType.getRawType(); if (rawType.equals(List.class) || rawType.equals(Set.class)) { var typeArgument = pType.getActualTypeArguments()[0]; - return typeArgument.equals(String.class) || (typeArgument instanceof Class && ((Class) typeArgument).isEnum()); + return typeArgument.equals(String.class) || + typeArgument.equals(Double.class) || + typeArgument.equals(Integer.class) || + (typeArgument instanceof Class && ((Class) typeArgument).isEnum()); } if (rawType.equals(Class.class)) @@ -412,6 +415,11 @@ private Object fromString(String value, Class type, @Nullable Field paramFiel List> enumConstants = getEnumConstants(paramField); return stream.map(s -> stringToEnumValue(s, enumConstants)).collect(toImmutableSet()); } + if (paramField != null && isCollectionOfDoubleType(paramField)) + return stream.map(Double::parseDouble).collect(toImmutableSet()); + if (paramField != null && isCollectionOfIntegerType(paramField)) + return stream.map(Integer::parseInt).collect(toImmutableSet()); + return stream.collect(toImmutableSet()); } else if (type.equals(List.class)) { if (value.isBlank()) { @@ -422,6 +430,11 @@ private Object fromString(String value, Class type, @Nullable Field paramFiel List> enumConstants = getEnumConstants(paramField); return stream.map(s -> stringToEnumValue(s, enumConstants)).toList(); } + if (paramField != null && isCollectionOfDoubleType(paramField)) + return stream.map(Double::parseDouble).toList(); + if (paramField != null && isCollectionOfIntegerType(paramField)) + return stream.map(Integer::parseInt).toList(); + return stream.toList(); } else if (type.equals(Class.class)) { try { @@ -488,7 +501,9 @@ private String getParamField(Field paramField) { boolean accessible = enforceAccessible(paramField); try { var result = paramField.get(this); - if (result != null && isCollectionOfEnumsWithUniqueStringValues(paramField)) { + if (result != null && (isCollectionOfEnumsWithUniqueStringValues(paramField) || + isCollectionOfDoubleType(paramField) || + isCollectionOfIntegerType(paramField))) { result = ((Collection) result).stream() .map(Object::toString) // map enum values to string .collect(Collectors.toList()); @@ -674,6 +689,30 @@ private static boolean isCollectionOfEnumsWithUniqueStringValues(Field paramFiel return false; } + private static boolean isCollectionOfIntegerType(Field paramField) { + var type = paramField.getGenericType(); + if (type instanceof ParameterizedType pType) { + var rawType = pType.getRawType(); + if (rawType.equals(List.class) || rawType.equals(Set.class)) { + var typeArgument = pType.getActualTypeArguments()[0]; + return typeArgument.equals(Integer.class) || typeArgument.equals(Integer.TYPE); + } + } + return false; + } + + private static boolean isCollectionOfDoubleType(Field paramField) { + var type = paramField.getGenericType(); + if (type instanceof ParameterizedType pType) { + var rawType = pType.getRawType(); + if (rawType.equals(List.class) || rawType.equals(Set.class)) { + var typeArgument = pType.getActualTypeArguments()[0]; + return typeArgument.equals(Double.class) || typeArgument.equals(Double.TYPE); + } + } + return false; + } + private static boolean enumStringsAreUnique(Class enumClass) { T[] enumConstants = enumClass.getEnumConstants(); long uniqueStringValues = Arrays.stream(enumConstants) diff --git a/matsim/src/main/java/org/matsim/run/gui/ExeRunner.java b/matsim/src/main/java/org/matsim/run/gui/ExeRunner.java index 8e2f94c3109..fac8a5df38d 100644 --- a/matsim/src/main/java/org/matsim/run/gui/ExeRunner.java +++ b/matsim/src/main/java/org/matsim/run/gui/ExeRunner.java @@ -97,6 +97,13 @@ public void killProcess() { public void run() { var processBuilder = new ProcessBuilder(); processBuilder.environment().put("MATSIM_GUI", "true"); // add "MATSIM_GUI" to the inherited vars + + // Copy the MATSIM_GUI_ARGS environment variable to the process environment + // these arguments may be used internally by the matsim scenario + if (System.getProperty("MATSIM_GUI_ARGS") != null) { + processBuilder.environment().put("MATSIM_GUI_ARGS", System.getProperty("MATSIM_GUI_ARGS")); + } + if (workingDirectory != null) { processBuilder.directory(new File(workingDirectory)); } diff --git a/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java b/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java index 17b8278da9e..9e0f1e55fb1 100644 --- a/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java +++ b/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java @@ -37,6 +37,7 @@ import org.matsim.api.core.v01.Coord; import org.matsim.api.core.v01.Id; import org.matsim.api.core.v01.network.Link; +import org.matsim.api.core.v01.population.Person; import org.matsim.core.config.ReflectiveConfigGroup.InconsistentModuleException; import org.matsim.testcases.MatsimTestUtils; @@ -70,6 +71,7 @@ void testDumpAndRead() { dumpedModule.enumSetField = Set.of(MyEnum.VALUE2); dumpedModule.setField = ImmutableSet.of("a", "b", "c"); dumpedModule.listField = List.of("1", "2", "3"); + dumpedModule.ints = List.of(1, 2, 3); assertEqualAfterDumpAndRead(dumpedModule); } @@ -166,6 +168,7 @@ void testComments() { expectedComments.put("enumListField", "list of enum"); expectedComments.put("enumSetField", "set of enum"); expectedComments.put("setField", "set"); + expectedComments.put("ints", "list of ints"); assertThat(new MyModule().getComments()).isEqualTo(expectedComments); } @@ -324,7 +327,7 @@ void testFailUnsupportedType_StringCollections() { void testFailUnsupportedType_NonStringList() { assertThatThrownBy(() -> new ReflectiveConfigGroup("name") { @Parameter("field") - private List stuff; + private List stuff; }).isInstanceOf(InconsistentModuleException.class); } @@ -472,6 +475,10 @@ private static class MyModule extends ReflectiveConfigGroup { @Parameter private Set enumSetField; + @Comment("list of ints") + @Parameter + private List ints; + // Object fields: // Id: string representation is toString private Id idField;