Skip to content

Commit

Permalink
a number of things that came up when auditing the leipzig scenario ge…
Browse files Browse the repository at this point in the history 
…neration. In particular:

* set the setCRS functionality in NetworkConfigGroup to deprecated (should be set directly in file)
* write Counts out in the simulated CRS as we do everywhere else, rather than projecting them back to the input CRS
  • Loading branch information
@kainagel
kainagel committed Feb 27, 2024
1 parent 2b80fee commit a881ad7
Show file tree
Hide file tree
Showing 11 changed files with 178 additions and 124 deletions.
19 changes: 13 additions & 6 deletions ...pplication/src/main/java/org/matsim/application/prepare/population/TrajectoryToPlans.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@
/**
* Creates the population from the original input data.
*
* "activity split by duration" refers to our convention to have activity types work_3600, work_7200 etc.
*
* "trajectory-to-plans" seems a bit mis-named because everything are plans. What it seems to do is to remove routes and modes.
*
* @author rakow
*/
@CommandLine.Command(
name = "trajectory-to-plans",
description = "Create population including, down-sampling, and activity split by duration",
description = "Create population including down-sampling, and activity split by duration",
showDefaultValues = true
)
public class TrajectoryToPlans implements MATSimAppCommand {
Expand Down Expand Up @@ -55,7 +59,7 @@ public class TrajectoryToPlans implements MATSimAppCommand {
@CommandLine.Option(names = {"--activity-bin-size", "--abs"}, description = "Activity types are extended so that they belong to a typical duration. This parameter influences the number of typical duration classes. The default is 600s")
private int activityBinSize = 600;

@CommandLine.Option(names = {"--max-typical-duration", "--mtd"}, description = "Max duration of activities for which a typical activity duration type is created in seconds. Default is 86400s (24h)")
@CommandLine.Option(names = {"--max-typical-duration", "--mtd"}, description = "Max duration of activities for which a typical activity duration type is created in seconds. Default is 86400s (24h). if set to 0, activities are not split.")
private int maxTypicalDuration = 86400;

@CommandLine.Option(names = "--output", description = "Output folder", defaultValue = "scenarios/input")
Expand Down Expand Up @@ -95,20 +99,23 @@ public Integer call() throws Exception {
v.getAttributes().putAttribute("subpopulation", "person");

});
// (if a <person/> does not yet have a subpopulation attribute, tag it as a "person". kai, feb'2024)

if (crs.getTargetCRS() != null) {
ProjectionUtils.putCRS(scenario.getPopulation(), crs.getTargetCRS());
log.info("Setting crs to: {}", ProjectionUtils.getCRS(scenario.getPopulation()));
}
// (if set by command line)

if (crs.getInputCRS() != null) {
ProjectionUtils.putCRS(scenario.getPopulation(), crs.getInputCRS());
log.info("Setting crs to: {}", ProjectionUtils.getCRS(scenario.getPopulation()));
}
// (if set by command line, this will overwrite the above targetCRS. How is this to be interpreted? kai, feb'2024)

if (maxTypicalDuration > 0) {
splitActivityTypesBasedOnDuration(scenario.getPopulation());
}
if (maxTypicalDuration > 0) {
splitActivityTypesBasedOnDuration(scenario.getPopulation());
}

PopulationUtils.writePopulation(scenario.getPopulation(),
output.resolve(String.format("%s-%dpct.plans.xml.gz", name, Math.round(sampleSize * 100))).toString());
Expand All @@ -134,7 +141,7 @@ public Integer call() throws Exception {
return 0;
}

@Deprecated
@Deprecated // why? what should I use instead? kai, feb'24
/**
* Split activities into typical durations to improve value of travel time savings calculation.
*/
Expand Down
8 changes: 8 additions & 0 deletions matsim/src/main/java/org/matsim/core/config/groups/CountsConfigGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,19 @@ public void setAnalyzedModes(final String analyzedModes) {
this.analyzedModes = analyzedModes.toLowerCase(Locale.ROOT);
}

/**
* @deprecated Coordinate System can now be set directly in file, which is the better place for this information, and thus the switch here is no longer needed. kai, feb'24
*/
@Deprecated // set directly in file.
@StringGetter( INPUT_CRS )
public String getInputCRS() {
return inputCRS;
}

/**
* @deprecated Coordinate System can now be set directly in file, which is the better place for this information, and thus the switch here is no longer needed. kai, feb'24
*/
@Deprecated // set directly in file.
@StringSetter( INPUT_CRS )
public void setInputCRS(String inputCRS) {
this.inputCRS = inputCRS;
Expand Down
13 changes: 11 additions & 2 deletions matsim/src/main/java/org/matsim/core/config/groups/NetworkConfigGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final class NetworkConfigGroup extends ReflectiveConfigGroup {

private String inputFile = null;

private String inputCRS = null;
@Deprecated private String inputCRS = null;

private String changeEventsInputFile = null;

Expand All @@ -57,7 +57,7 @@ public NetworkConfigGroup() {
public Map<String,String> getComments() {
final Map<String,String> comments = super.getComments();

comments.put( INPUT_CRS , "The Coordinates Reference System in which the coordinates are expressed in the input file." +
comments.put( INPUT_CRS , "(deprecated: rather express CRS in file) The Coordinates Reference System in which the coordinates are expressed in the input file." +
" At import, the coordinates will be converted to the coordinate system defined in \"global\", and will" +
"be converted back at export. If not specified, no conversion happens." );

Expand Down Expand Up @@ -121,11 +121,20 @@ public String getLaneDefinitionsFile(){
}


/**
* @deprecated Coordinate System can now be set directly in file, which is the better place for this information, and thus the switch here is no longer needed. kai, feb'24
*/
@Deprecated // set directly in file.
@StringGetter( INPUT_CRS )
public String getInputCRS() {
return inputCRS;
}
// I think that this should be deprecated since the same functionality can be achieved by writing it directly into the corresponding file. kai, feb'24

/**
* @deprecated Coordinate System can now be set directly in file, which is the better place for this information, and thus the switch here is no longer needed. kai, feb'24
*/
@Deprecated // set directly in file
@StringSetter( INPUT_CRS )
public void setInputCRS(String inputCRS) {
this.inputCRS = inputCRS;
Expand Down
33 changes: 19 additions & 14 deletions matsim/src/main/java/org/matsim/core/controler/corelisteners/DumpDataAtEndImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ final class DumpDataAtEndImpl implements DumpDataAtEnd, ShutdownListener {

@Inject
private Map<Class<?>,AttributeConverter<?>> attributeConverters = Collections.emptyMap();
// (yyyy Feels plausible to have them but how can they be set? And isn't there a more global way to set the attribute converters? kai, feb'24)

@Override
public void notifyShutdown(ShutdownEvent event) {
Expand Down Expand Up @@ -214,22 +215,26 @@ private void dumpExperiencedPlans(int iteration) {
private void dumpCounts() {
try {
if (this.counts != null ) {
final String inputCRS = this.config.counts().getInputCRS();
final String internalCRS = this.config.global().getCoordinateSystem();
// final String inputCRS = this.config.counts().getInputCRS();
// final String internalCRS = this.config.global().getCoordinateSystem();

if ( inputCRS == null ) {
// if ( inputCRS == null ) {
new CountsWriter(this.counts).write(this.controlerIO.getOutputFilename(Controler.DefaultFiles.counts));
}
else {
log.info( "re-projecting counts from "+internalCRS+" back to "+inputCRS+" for export" );

final CoordinateTransformation transformation =
TransformationFactory.getCoordinateTransformation(
internalCRS,
inputCRS );

new CountsWriter( transformation , this.counts).write(this.controlerIO.getOutputFilename(Controler.DefaultFiles.counts));
}
// }
// else {
// log.info( "re-projecting counts from "+internalCRS+" back to "+inputCRS+" for export" );
//
// final CoordinateTransformation transformation =
// TransformationFactory.getCoordinateTransformation(
// internalCRS,
// inputCRS );
//
// new CountsWriter( transformation , this.counts).write(this.controlerIO.getOutputFilename(Controler.DefaultFiles.counts));
// }
// we said at some point that we are no longer projecting back for the final output. For Counts, this was so far not
// adapted in this direction. I assume that the reason was that the CountsWriter took a transformation, not the
// coordinate string itself, and so it was not possible to automatically at the CRS string as attribute into the file.
// I adapted that now (I hope). kai, feb'24
}
} catch ( Exception ee ) {
log.error("Exception writing counts.", ee);
Expand Down
37 changes: 13 additions & 24 deletions matsim/src/main/java/org/matsim/core/network/io/NetworkReaderMatsimV2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
import java.util.Set;
import java.util.Stack;

import static org.matsim.utils.objectattributes.attributable.AttributesXmlReaderDelegate.TAG_ATTRIBUTE;
import static org.matsim.utils.objectattributes.attributable.AttributesXmlReaderDelegate.TAG_ATTRIBUTES;

/**
* A reader for network-files of MATSim according to <code>network_v1.dtd</code>.
*
Expand All @@ -55,9 +58,6 @@ final class NetworkReaderMatsimV2 extends MatsimXmlParser {
private final static String LINKS = "links";
private final static String NODE = "node";
private final static String LINK = "link";
private final static String ATTRIBUTES = "attributes";
private final static String ATTRIBUTE = "attribute";

private final Network network;

private final AttributesXmlReaderDelegate attributesDelegate = new AttributesXmlReaderDelegate();
Expand Down Expand Up @@ -85,32 +85,21 @@ final class NetworkReaderMatsimV2 extends MatsimXmlParser {

@Override
public void startTag(final String name, final Attributes atts, final Stack<String> context) {
switch( name ) {
case NODE:
startNode(atts);
break;
case LINK:
startLink(atts);
break;
case NETWORK:
startNetwork(atts);
break;
case LINKS:
startLinks(atts);
break;
case ATTRIBUTES:
/* fall-through */
case ATTRIBUTE:
attributesDelegate.startTag( name , atts , context , currentAttributes );
break;
switch( name ){
case NODE -> startNode( atts );
case LINK -> startLink( atts );
case NETWORK -> startNetwork( atts );
case LINKS -> startLinks( atts );
case TAG_ATTRIBUTES, TAG_ATTRIBUTE -> attributesDelegate.startTag( name, atts, context, currentAttributes );
default -> throw new IllegalStateException( "Unexpected value: " + name );
}
}

@Override
public void endTag(final String name, final String content, final Stack<String> context) {
switch( name ) {
case ATTRIBUTES:
if (context.peek().equals(NETWORK)) {
case TAG_ATTRIBUTES:
if (context.peek().equals(NETWORK)) {
String inputCRS = (String) network.getAttributes().getAttribute(ProjectionUtils.INPUT_CRS_ATT);
if (inputCRS != null && targetCRS != null) {
if (externalInputCRS != null) {
Expand All @@ -122,7 +111,7 @@ public void endTag(final String name, final String content, final Stack<String>
}
}
/* fall-through */
case ATTRIBUTE:
case TAG_ATTRIBUTE:
attributesDelegate.endTag(name, content, context);
break;
}
Expand Down
9 changes: 8 additions & 1 deletion matsim/src/main/java/org/matsim/counts/Counts.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import org.matsim.api.core.v01.Id;
import org.matsim.api.core.v01.Identifiable;
import org.matsim.core.api.internal.MatsimFactory;
import org.matsim.core.api.internal.MatsimToplevelContainer;
import org.matsim.utils.objectattributes.attributable.Attributable;
import org.matsim.utils.objectattributes.attributable.Attributes;
import org.matsim.utils.objectattributes.attributable.AttributesImpl;
Expand All @@ -16,7 +18,7 @@
* to an identifiable object (links, nodes, transit stations e.g)
* Structure is similar to regular counts, but more flexible to use.
*/
public final class Counts<T extends Identifiable<T>> implements Attributable {
public final class Counts<T extends Identifiable<T>> implements Attributable, MatsimToplevelContainer {

public static final String ELEMENT_NAME = "counts";
private final Map<Id<T>, MeasurementLocation<T>> locations = new TreeMap<>();
Expand Down Expand Up @@ -149,4 +151,9 @@ public Attributes getAttributes() {
public String toString() {
return "[name=" + this.name + "]" + "[nof_locations=" + this.locations.size() + "]";
}

@Override public MatsimFactory getFactory(){
throw new RuntimeException( "not implemented" );
// yy I need this to fulfill MatsimToplevelContainer. Which I need to be able to use ProjectionUtils.putCRS(...) in the readers/writers. kai, feb'24
}
}
13 changes: 4 additions & 9 deletions matsim/src/main/java/org/matsim/counts/CountsModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,13 @@ Counts<Link> provideLinkCounts(Scenario scenario, CountsConfigGroup config) {
MatsimCountsReader counts_parser;
if (inputCRS == null) {
counts_parser = new MatsimCountsReader(counts);
}
}
else {
log.info( "re-projecting counts from "+inputCRS+" to "+internalCRS+" for import" );

final CoordinateTransformation transformation =
TransformationFactory.getCoordinateTransformation(
inputCRS,
internalCRS );
log.info( "re-projecting counts from "+inputCRS+" to "+internalCRS+" for import" );

counts_parser = new MatsimCountsReader( transformation , counts );
counts_parser = new MatsimCountsReader( inputCRS, internalCRS, counts );
}
counts_parser.parse(config.getCountsFileURL(scenario.getConfig().getContext()));
counts_parser.parse(config.getCountsFileURL(scenario.getConfig().getContext()));
}
return counts;
}
Expand Down
51 changes: 40 additions & 11 deletions matsim/src/main/java/org/matsim/counts/CountsReaderMatsimV2.java
View file
Original file line number Diff line number Diff line change
@@ -1,58 +1,87 @@
package org.matsim.counts;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.matsim.api.core.v01.Coord;
import org.matsim.api.core.v01.Id;
import org.matsim.api.core.v01.Identifiable;
import org.matsim.core.scenario.ProjectionUtils;
import org.matsim.core.utils.geometry.CoordinateTransformation;
import org.matsim.core.utils.geometry.transformations.IdentityTransformation;
import org.matsim.core.utils.geometry.transformations.TransformationFactory;
import org.matsim.core.utils.io.MatsimXmlParser;
import org.matsim.utils.objectattributes.attributable.AttributesXmlReaderDelegate;
import org.xml.sax.Attributes;

import java.util.Stack;

import static org.matsim.utils.objectattributes.attributable.AttributesXmlReaderDelegate.TAG_ATTRIBUTE;
import static org.matsim.utils.objectattributes.attributable.AttributesXmlReaderDelegate.TAG_ATTRIBUTES;

/**
* A reader for counts-files of MATSim according to <code>counts_v2.xsd</code>.
*/
public class CountsReaderMatsimV2 extends MatsimXmlParser {

private static final Logger log = LogManager.getLogger( CountsReaderMatsimV2.class );
private final static String VALUE = "value";
private final static String ATTRIBUTES = "attributes";

private final Class<? extends Identifiable<?>> idClass;
private final CoordinateTransformation coordinateTransformation;
private CoordinateTransformation coordinateTransformation;
private final Counts<?> counts;
private final AttributesXmlReaderDelegate attributesDelegate = new AttributesXmlReaderDelegate();
private final String externalInputCRS;
private final String targetCRS;
private MeasurementLocation<?> currLocation;
private Measurable currMeasurable;
private org.matsim.utils.objectattributes.attributable.Attributes currAttributes = null;

public CountsReaderMatsimV2(Counts<?> counts, Class<? extends Identifiable<?>> idClass) {
this(new IdentityTransformation(), counts, idClass);
this( null, null, counts, idClass );
}

public CountsReaderMatsimV2(CoordinateTransformation coordinateTransformation, Counts<?> counts, Class<? extends Identifiable<?>> idClass) {
public CountsReaderMatsimV2( String externalInputCRS, String targetCRS, Counts<?> counts, Class<? extends Identifiable<?>> idClass ) {
super(ValidationType.NO_VALIDATION);
this.coordinateTransformation = coordinateTransformation;
this.externalInputCRS = externalInputCRS;
this.targetCRS = targetCRS;
this.counts = counts;
this.idClass = idClass;

if (externalInputCRS != null && targetCRS != null) {
this.coordinateTransformation = TransformationFactory.getCoordinateTransformation(externalInputCRS, targetCRS);
ProjectionUtils.putCRS(this.counts, targetCRS);
}

}

@Override
public void startTag(String name, Attributes atts, Stack<String> context) {

switch (name) {
case Counts.ELEMENT_NAME -> startMultiModeCounts(atts);
case MeasurementLocation.ELEMENT_NAME -> startMeasurementLocation(atts);
case Measurable.ELEMENT_NAME -> startMeasurable(name, atts);
case ATTRIBUTES -> attributesDelegate.startTag(name, atts, context, currAttributes);
case TAG_ATTRIBUTES, TAG_ATTRIBUTE -> attributesDelegate.startTag(name, atts, context, currAttributes);
case VALUE -> addValuesToMeasurable(atts);
}
}

@Override
public void endTag(String name, String content, Stack<String> context) {

switch( name ) {
case TAG_ATTRIBUTES:
if (context.peek().equals(Counts.ELEMENT_NAME)) {
String inputCRS = (String) counts.getAttributes().getAttribute( ProjectionUtils.INPUT_CRS_ATT );
if (inputCRS != null && targetCRS != null) {
if (externalInputCRS != null) {
// warn or crash?
log.warn("coordinate transformation defined both in config and in input file: setting from input file will be used");
}
coordinateTransformation = TransformationFactory.getCoordinateTransformation(inputCRS, targetCRS );
ProjectionUtils.putCRS(counts, targetCRS);
}
}
/* fall-through */
case TAG_ATTRIBUTE:
attributesDelegate.endTag(name, content, context);
break;
}
}

private void addValuesToMeasurable(Attributes atts) {
Expand Down
Loading

0 comments on commit a881ad7

Please sign in to comment.