Merge pull request matsim-org#3344 from matsim-org/bicycleContrib

* repair wrong positioning of vehicles if linkSpeedCalculator returns…

contribs/bicycle/src/main/java/org/matsim/contrib/bicycle/BicycleLinkSpeedCalculatorDefaultImpl.java

@@ -18,7 +18,10 @@ public final class BicycleLinkSpeedCalculatorDefaultImpl implements BicycleLinkS
 	private static final Logger log = LogManager.getLogger(BicycleLinkSpeedCalculatorDefaultImpl.class );
 	@Inject private BicycleConfigGroup bicycleConfigGroup;
 	@Inject private QSimConfigGroup qSimConfigGroup;
-	@Inject private BicycleLinkSpeedCalculatorDefaultImpl() { }
+	@Inject private Config config;
+	@Inject private BicycleLinkSpeedCalculatorDefaultImpl() {
+	}
+
 	/**
 	 * for unit testing
 	 */

contribs/bicycle/src/main/java/org/matsim/contrib/bicycle/BicycleModule.java

@@ -22,8 +22,12 @@
 import com.google.inject.Singleton;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.checkerframework.checker.units.qual.C;
 import org.matsim.api.core.v01.Id;
 import org.matsim.api.core.v01.Scenario;
+import org.matsim.core.config.Config;
+import org.matsim.core.config.ConfigGroup;
+import org.matsim.core.config.ConfigUtils;
 import org.matsim.core.controler.AbstractModule;
 import org.matsim.core.controler.events.StartupEvent;
 import org.matsim.core.controler.listener.StartupListener;
@@ -34,37 +38,51 @@
  * @author smetzler, dziemke
  */
 public final class BicycleModule extends AbstractModule {
-
 	private static final Logger LOG = LogManager.getLogger(BicycleModule.class);
 
-	@Inject
-	private BicycleConfigGroup bicycleConfigGroup;
+	@Inject private BicycleConfigGroup bicycleConfigGroup;
+
+	@Override public void install() {
+//		BicycleConfigGroup bicycleConfigGroup = ConfigUtils.addOrGetModule( this.getConfig(), BicycleConfigGroup.class );
+//		this.bind( BicycleConfigGroup.class ).toInstance( bicycleConfigGroup );
+		// the above feels odd.  But it seems to work.  I actually have no idea where the config groups are bound, neither for the core config
+		// groups nor for the added config groups.  In general, the original idea was that AbstractModule provides the config from
+		// getConfig(), not from injection.  kai, jun'24
+
+		// It actually does not work in general.  The ExplodedConfigModule injects all config groups that are materialized by then.  Which
+		// means that it needs to be materialized "quite early", and in particular before this install method is called.  For the time being,
+		// a run script using the contrib thus needs to materialize the config group.  kai, jul'24
+
 
-	@Override
-	public void install() {
 		// The idea here is the following:
 		// * scores are just added as score events.  no scoring function is replaced.
 
 		// * link speeds are computed via a plugin handler to the DefaultLinkSpeedCalculator.  If the plugin handler returns a speed, it is
 		// used, otherwise the default speed is used. This has the advantage that multiple plugins can register such special link speed calculators.
 
+		// this gives the typical things to the router:
 		addTravelTimeBinding(bicycleConfigGroup.getBicycleMode()).to(BicycleTravelTime.class).in(Singleton.class);
 		addTravelDisutilityFactoryBinding(bicycleConfigGroup.getBicycleMode()).to(BicycleTravelDisutilityFactory.class).in(Singleton.class);
+		// (the BicycleTravelTime uses the BicycleLinkSpeed Calculator bound below)
+		// (the BicycleDisutility uses a BicycleTravelDisutility)
 
+		// compute and throw the additional score events:
 		this.addEventHandlerBinding().to( BicycleScoreEventsCreator.class );
-		// (the motorized interaction is in the BicycleScoreEventsCreator)
+		// (this uses the AdditionalBicycleLinkScore to compute and throw corresponding scoring events)
+		// (it also computes and throws the motorized interaction events, if they are switched on)
 
 		this.bind( AdditionalBicycleLinkScore.class ).to( AdditionalBicycleLinkScoreDefaultImpl.class );
-
-		bind( BicycleLinkSpeedCalculator.class ).to( BicycleLinkSpeedCalculatorDefaultImpl.class ) ;
-		// this is still needed because the bicycle travel time calculator for routing needs to use the same bicycle speed as the mobsim.  kai, jun'23
+		// (this computes the value of the per-link scoring event.  yyyy Very unfortunately, it is a re-implementation of the BicycleTravelDisutility (mentioned above).)
 
 		this.installOverridingQSimModule( new AbstractQSimModule(){
 			@Override protected void configureQSim(){
 				this.addLinkSpeedCalculator().to( BicycleLinkSpeedCalculator.class );
 			}
 		} );
 
+		bind( BicycleLinkSpeedCalculator.class ).to( BicycleLinkSpeedCalculatorDefaultImpl.class ) ;
+		// (both the router and the mobsim need this)
+
 		addControlerListenerBinding().to(ConsistencyCheck.class);
 	}
 
@@ -73,7 +91,6 @@ static class ConsistencyCheck implements StartupListener {
 		@Inject private Scenario scenario;
 
 		@Override public void notifyStartup(StartupEvent event) {
-
 			Id<VehicleType> bicycleVehTypeId = Id.create(bicycleConfigGroup.getBicycleMode(), VehicleType.class);
 			if (scenario.getVehicles().getVehicleTypes().get(bicycleVehTypeId) == null) {
 				LOG.warn("There is no vehicle type '" + bicycleConfigGroup.getBicycleMode() + "' specified in the vehicle types. "

contribs/bicycle/src/main/java/org/matsim/contrib/bicycle/BicycleTravelDisutilityFactory.java

@@ -21,6 +21,8 @@
 import com.google.inject.Inject;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.matsim.core.config.Config;
+import org.matsim.core.config.ConfigUtils;
 import org.matsim.core.config.groups.ScoringConfigGroup;
 import org.matsim.core.config.groups.RoutingConfigGroup;
 import org.matsim.core.router.costcalculators.TravelDisutilityFactory;
@@ -35,21 +37,20 @@
  */
 public final class BicycleTravelDisutilityFactory implements TravelDisutilityFactory {
 	// public-final is ok since ctor is package-private: can only be used through injection
-
 	private static final Logger LOG = LogManager.getLogger(BicycleTravelDisutilityFactory.class);
-
-	@Inject	BicycleConfigGroup bicycleConfigGroup;
-	@Inject
-	ScoringConfigGroup cnScoringGroup;
-	@Inject
-	RoutingConfigGroup routingConfigGroup;
-
+	private BicycleConfigGroup bicycleConfigGroup;
+	@Inject Config config;
+	@Inject ScoringConfigGroup cnScoringGroup;
+	@Inject RoutingConfigGroup routingConfigGroup;
 	private static int normalisationWrnCnt = 0;
 
-	/* package-private */ BicycleTravelDisutilityFactory(){}
+	/* package-private */ BicycleTravelDisutilityFactory(){
+	}
 
 	@Override
 	public TravelDisutility createTravelDisutility(TravelTime timeCalculator) {
+		this.bicycleConfigGroup = ConfigUtils.addOrGetModule( config, BicycleConfigGroup.class );
+
 		double sigma = routingConfigGroup.getRoutingRandomness();
 
 		double normalization = 1;

contribs/bicycle/src/main/java/org/matsim/contrib/bicycle/BicycleUtils.java

@@ -127,4 +127,11 @@ private static boolean hasNoCycleway( String cyclewayType ) {
 		}
 		return userDefinedNetworkAttributeFactor;
 	}
+	// ===
+	public static void setSmoothness( Link link, String smoothness ){
+		link.getAttributes().putAttribute( SMOOTHNESS, smoothness );
+	}
+	public static void setBicycleInfrastructureFactor( Link link, double factor ){
+		link.getAttributes().putAttribute( BICYCLE_INFRASTRUCTURE_SPEED_FACTOR, factor );
+	}
 }

contribs/bicycle/src/main/java/org/matsim/contrib/bicycle/network/BicycleOsmNetworkReaderV2.java

@@ -225,7 +225,7 @@ protected void setOrModifyLinkAttributes(Link l, OsmWay way, boolean forwardDire
 		// Smoothness
 		String smoothness = way.tags.get(BicycleUtils.SMOOTHNESS);
 		if (smoothness != null) {
-			l.getAttributes().putAttribute(BicycleUtils.SMOOTHNESS, smoothness);
+			BicycleUtils.setSmoothness( l, smoothness );
 			this.countSmoothness++;
 		}
 

matsim/src/main/java/org/matsim/core/controler/ControlerDefaultsModule.java

@@ -85,8 +85,7 @@ public void install() {
 		// Maybe not the best place to but this but since ChartUtils is used by many modules, including default ones,
 		//  the cache needs to be always set correctly.
 		addControlerListenerBinding().toInstance(new StartupListener() {
-			@Inject
-			private OutputDirectoryHierarchy outputDirectoryHierarchy;
+			@Inject private OutputDirectoryHierarchy outputDirectoryHierarchy;
 			@Override   public void notifyStartup(StartupEvent event) {
 				ImageIO.setCacheDirectory(new File(outputDirectoryHierarchy.getTempPath()));
 			}

matsim/src/main/java/org/matsim/core/mobsim/qsim/qnetsimengine/AbstractAgentSnapshotInfoBuilder.java

@@ -139,12 +139,13 @@ public final void positionAgentGivenDistanceFromFNode(final Collection<AgentSnap
 		positions.add(position);
 	}
 
-	public final Collection<AgentSnapshotInfo> positionVehiclesAlongLine(Collection<AgentSnapshotInfo> positions,
-			double now, Collection<? extends MobsimVehicle> vehs, double curvedLength, double storageCapacity,
-			Coord upstreamCoord, Coord downstreamCoord, double inverseFlowCapPerTS, double freeSpeed,
-			int numberOfLanesAsInt, Queue<Hole> holes)
+	public final Collection<AgentSnapshotInfo> positionVehiclesAlongLine( Collection<AgentSnapshotInfo> positions,
+									      double now, Collection<? extends MobsimVehicle> vehs, double curvedLength, double storageCapacity,
+									      Coord upstreamCoord, Coord downstreamCoord, double inverseFlowCapPerTS, double freeSpeed,
+									      int numberOfLanesAsInt, Queue<Hole> holes, AbstractQLink.QLinkInternalInterface qLinkInternalInterface )
 	{
 		double spacingOfOnePCE = this.calculateVehicleSpacing( curvedLength, storageCapacity, vehs );
+		// ("vehs" is needed since the link may be more than full because of squeezing.  In this case, spacingOfOnePCE is smaller than one "cell".)
 
 		double ttimeOfHoles = curvedLength / (QueueWithBuffer.HOLE_SPEED_KM_H*1000./3600.);
 
@@ -211,8 +212,16 @@ public final Collection<AgentSnapshotInfo> positionVehiclesAlongLine(Collection<
 			// (starts off relatively small (rightmost vehicle))
 
 			final double vehicleSpacing = mveh.getSizeInEquivalents() * spacingOfOnePCE;
+
+			double speed = min( freeSpeed, veh.getMaximumVelocity() );
+			if ( qLinkInternalInterface!=null ){
+				speed = qLinkInternalInterface.getMaximumVelocityFromLinkSpeedCalculator( veh, now );
+			}
+
 			distanceFromFromNode = this.calculateOdometerDistanceFromFromNode(
-					now, curvedLength, min( freeSpeed, veh.getMaximumVelocity()), vehicleSpacing, distanceFromFromNode, remainingTravelTime
+					now, curvedLength,
+					speed, // min( freeSpeed, veh.getMaximumVelocity()),
+					vehicleSpacing, distanceFromFromNode, remainingTravelTime
 			);
 			// yyyy if the LinkSpeedCalculator says something that is not free speed, we are out of luck here.  kai, jan'23