matsim-libs
matsim-libs copied to clipboard
MATSim 13.0: Incompatibility of SwissRailRaptor with custom MainModeIdentifier
MATSim 13.0 might have an incompatiblity issue:
If somebody uses transit and configures the RoutingAlgorithmType
to be SwissRailRaptor
, the SwissRailRaptorModule
is automatically bound (since commit e242664b). In Versions before commit c7f1761a though, the SwissRailRaptorModule
binds its own MainModeIdentifier, i.e. in the MATSim 13.0 release.
Here is the thing (which i still do not fully understand):
The user can not override the binding of the MainModeIdentifier
(in the usual way of declaring adding an overriding module to the Controler
). If one tries that, an exception is thrown inside the SwissRailRaptor at the line of the MainModeIdentifier binding, saysing that this interface has already bound at another place. To me, it seems like the binding of the overriding module was processed before the binding in the SwissRailRaptorModule
. This might be, because the SwissRailRaptorModule
is now part of the ControlerDefaultsModule
pipeline. But i might be totally wrong: Guice does its own magic.
I have tested various setups and runs (within matsim-scnearios/matsim-hamburg) and came to the point that for MATSim versions between the above mentioned commits, one can probably not use the SwissRailRaptorModule
with another MainModeIdentifier
. Now, drt uses its own MainModeIdentifier
, too. As do probably more modules and scenarios. If that suspicion is correct, that means that all of the (nowadays many) scenarios relying on the SwissRailRaptor
can not be run with drt, when using MATSim 13.0. I have checked the RunDRTExampleTestIT, and it understandably does not use transit. I am not sure whether there is a test combining the SwissRailRaptor
with a custom MainModeIdentifier
inside of MATSim. It might be, that this is why this issue has found its way into the release.
Just some notes:
The custom MainModeIdentifier
was only bound by SwissRailRaptor if intermodalAccessEgress was enabled. This should at least limit the number of affected people a bit.
As for why it's not possible to overwrite it, I don't know. I would have expected that it is possible, but I'm no expert on that DI stuff.
With the introduction of the routingMode
attribute, the MainModeIdentifier should be used less and less, I think only for backwards compatibility (e.g. reading old plans files without the routingMode attribute) or for analysis. So I would expect that in many cases a custom MainModeIdentifier is no longer necessary for new code (unless the DRT contrib uses the MainModeIdentifier internally somehow which I'm not aware of).
It might make sense to back port commit c7f1761
to the 13.x
branch, so this fix will be included in an upcoming 13.1 release.
The custom
MainModeIdentifier
was only bound by SwissRailRaptor if intermodalAccessEgress was enabled. This should at least limit the number of affected people a bit.
Absolutely right. Thanks for pointing that out.
As for why it's not possible to overwrite it, I don't know. I would have expected that it is possible, but I'm no expert on that DI stuff.
Yes, i still can't wrap my head around that. I wonder if somebody else can?
With the introduction of the
routingMode
attribute, the MainModeIdentifier should be used less and less, I think only for backwards compatibility (e.g. reading old plans files without the routingMode attribute) or for analysis. So I would expect that in many cases a custom MainModeIdentifier is no longer necessary for new code (unless the DRT contrib uses the MainModeIdentifier internally somehow which I'm not aware of).
Again, absolutely right. So that limits the number of affected people again. But one has to know, that a MainModeIdentifier is no longer necessary..
It might make sense to back port commit
c7f1761
to the13.x
branch, so this fix will be included in an upcoming 13.1 release.
I cherry-picked that commit in #1564. Will a 13.1 release definetely come?
i merged #1564 such that c7f1761 is in the 13.x
branch. Will leave this issue open though, until 13.1 is released.