From 2b4d9201d66b027531be411eadb801a6b8a0be94 Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Fri, 31 Mar 2017 10:37:57 +0200 Subject: [PATCH 01/14] Turn restrictions works now on fromEdges which are split If fromedge in turnRestrictions are split in getOrCreateVertexNear then turn restrictions don't work. This can happen when linking P+R nodes, bike shares and transit stops. This adds tests and updates getOrCreateVertexNear so that split edges have turn restrictions same as when linking P+R areas. --- .../com/conveyal/r5/streets/StreetLayer.java | 6 + .../conveyal/r5/streets/TurnSplitTest.java | 119 ++++++++++++++++++ .../streets/turn-restriction-split-test.pbf | Bin 0 -> 6847 bytes 3 files changed, 125 insertions(+) create mode 100644 src/test/java/com/conveyal/r5/streets/TurnSplitTest.java create mode 100644 src/test/resources/com/conveyal/r5/streets/turn-restriction-split-test.pbf diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 0a2a4b46e..1c054706d 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -1139,6 +1139,12 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) EdgeStore.Edge newEdge1 = edgeStore.addStreetPair(newVertexIndex, oldToVertex, split.distance1_mm, edge.getOSMID()); // Copy the flags and speeds for both directions, making newEdge1 like the existing edge. newEdge1.copyPairFlagsAndSpeeds(edge); + + // clean up any turn restrictions that exist + // turn restrictions on the forward edge go to the new edge's forward edge. Turn restrictions on the back edge stay + // where they are + edgeStore.turnRestrictions.removeAll(split.edge).forEach(ridx -> edgeStore.turnRestrictions.put(newEdge1.edgeIndex, ridx)); + // Insert the new edge into the spatial index if (!edgeStore.isExtendOnlyCopy()) { spatialIndex.insert(newEdge1.getEnvelope(), newEdge1.edgeIndex); diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java new file mode 100644 index 000000000..e8dc3db52 --- /dev/null +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -0,0 +1,119 @@ +package com.conveyal.r5.streets; + +import com.conveyal.osmlib.OSM; +import com.conveyal.r5.point_to_point.builder.TNBuilderConfig; +import com.conveyal.r5.profile.ProfileRequest; +import com.conveyal.r5.profile.StreetMode; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Created by mabu on 29.3.2017. + */ +public class TurnSplitTest { + private static final Logger LOG = LoggerFactory.getLogger(TurnSplitTest.class); + + private static StreetLayer sl; + + @Before + public void setUpGraph() throws Exception { + OSM osm = new OSM(null); + osm.intersectionDetection = true; + osm.readFromUrl(StreetLayerTest.class.getResource("turn-restriction-split-test.pbf").toString()); + + sl = new StreetLayer(TNBuilderConfig.defaultConfig()); + // load from OSM and don't remove floating subgraphs + sl.loadFromOsm(osm, false, true); + + sl.buildEdgeLists(); + + } + + //Test if turn restriction still works if fromEdge is split + //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops + @Test + public void testTurnRestrictionWithSplit() throws Exception { + + ProfileRequest profileRequest = new ProfileRequest(); + profileRequest.fromLat = 38.8930088; + profileRequest.fromLon = -76.998343; + profileRequest.toLat = 38.8914185; + profileRequest.toLon = -76.9962294; + + StreetRouter streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + StreetRouter.State lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + + int oldWeight = lastState.weight; + StreetRouter.State state = lastState; + EdgeStore.Edge edge = sl.edgeStore.getCursor(); + + TurnRestriction tr = sl.turnRestrictions.get(1); + + LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getFlagsAsString()); + //Turn from 143 to 145 is in Turn restriction + assertFalse(state.backEdge == 134 && (state.backState.backEdge == 152 || state.backState.backEdge == 146)); + state = state.backState; + + } + + //Splits "from Turn restriction edge" + int vertex = sl.createAndLinkVertex(38.8921836,-76.9959183); + + sl.buildEdgeLists(); + + streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + state = lastState; + + int newWeight = lastState.weight; + + assertEquals(oldWeight, newWeight); + + + //LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + //LOG.info("V:{} W:{} be:{}, flags:{}", state.vertex, state.weight, state.backEdge, edge.getFlagsAsString()); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getFlagsAsString()); + //Turn from 143 to 146 is in Turn restriction + assertFalse(state.backEdge == 134 && (state.backState.backEdge == 152 || state.backState.backEdge == 146)); + state = state.backState; + + } + + } +} diff --git a/src/test/resources/com/conveyal/r5/streets/turn-restriction-split-test.pbf b/src/test/resources/com/conveyal/r5/streets/turn-restriction-split-test.pbf new file mode 100644 index 0000000000000000000000000000000000000000..ffe9a21bfe6c66cb518b0b589cacd39db4444b81 GIT binary patch literal 6847 zcmV;w8bIX$000gO2~Sf^NM&JUWpWsm0T71)8jk^Zoa2(=SiiUX_Ffi&?^h39*~=m^ z;n~s03z-xiJZOKokV%Qpzc@EIIU_YUQ8&y$&rFHSB{i=&)h|CKwOC6qIKQYwFQl>{ zHNGSxH80-NrHN4>zc@EJKd&scs6@d)&qB|%i%}(`q@=(~U%$M(Tra->sJ)~pHMJx+ zu|O}sC|y6XAX6V`FaUrPGXMYp01OHTPg6}qVRT^_`xp@NB^vb@c$}qG2~<;88ou|w zCC`@-LK1`GMx_eHsoJTwwJ;s)nI1b%)sAha?M&y`2vHCL1*~PpnL|O5B?_{M2!iZl zz#xc-U}X_mB1E9bCSpNl7lEQE=-l@rf>ux4Gk@N{egArIa$eX}!~*;gc(}PcJ9;>< z#=Bfy?YFD!oHlND{C1P82TR-S*r{^&USF$pqrJ1zg`KuV<+{mD>03@?Q~L^uVF>&KKt|& zyzr{^AAS0Xw8TPMXu(DhUWb^dVa<}xHxVL95|PZ0{wU2FVIn!eTTathaZEC_NFos# zvPeYJRTkJnv>y`XAao!bdeP^b%2q#WhEMz)?BM4y^m^t@&nh<{l8I=4QpWhPM z!lQDQTT0V>Jjq z8BP$~V=(bR;we#_nBq^Qa|NWqovgzU;u?{>LsAq?Q50@LO=&(kh&T9_+yj3l;W}RN zmcf1|Jn`^0)&wzu|7LowQaVH-o%cu#+Y?*wGOE*{vO|UBCCuOfc6wWi_QHdew|6%l zY-VyuUkLol!_Z#LzRzjE84<;$^tHfdroSu<^ZZQSS)}fgr{UOHqVoTnob^ z@-;A|TfZMxcVUqdT1W;|YLGwXyc^y-&SD0s7~}!~s4=_`b29dlFtfcx>5 z0O0QmfUK+DJJe;FqMk>4*wGRV(yI%}YBhJk*x`Vp{e@)Dr7)CJfiL?sCwL3+W=!Kv)G{6w6t4X6K4uU@a zTuN$Wv3XKOia9VK`qs4$lUY!!d@ffX>Hq;LnNv=FB;F3P)zEt4id7 zm~jwi;hLiKe4|3l6Scqq8$7QDA8D{QnhNrZGd&+201GfTWPLY1x9%uvtvyKh;jTsZ zk`;xR4RsaAdZrzXt0X{E&y$M;T=)Cw(eUAo-Ot>Q!NHqz7j+ghrvYcY;64C%yqwrQ zQUFe4IdW!Gl)ga0?S(sq<$IZC9Xqdh*^lxmov&U15^r%|by|`}bRft_2D+~5=3+m% zK{q!DGjA*a3xUBITn>P)K2$i_B5hP7JB}F)S47Mn!aRv!ngCNvyQZaJ<^jN`J&87F zmz#o|)i2&JYB-GmQ9u2Wcak4#;dASUCsJ$P7>p={VO4)gKzFzr0V9_Qdb zfld=+CPqCp3D?LB{c&24e&P4sa$M3HV0A6P3V?voIqHzLPb+Y>_5geG06V3{Z7swS z+@5-B4&adpjSTnaR8Z9H7hm2K3O*v?wO6ZM?YS?&<4_CE3A5k~thkT?19jE^o-8ZT z*1%%Spsm*jUB^6$V446Qw|?$*{d3+0eA1KH{!4u?eJy4*If9IsS-hP-=Jsr`{`h`e zcq{6wGufx7oc(HVHukJF+`l*vAbRR|$xJ#T?u!!}{cNfP7z3$^#2A}!ay2BCF#-t* zO=g$^U^Z(KpZ(P4{%odff@>j=%Yd8(zeLk{Ic+}wuAS(@&$gIMTDSmMS#lDSm8D`j zumYC6Z2?|F1X#>#(`R8b+JqaXTxpf`?O61OtZqK&x!wr+Xn+A-?{KdM32 zxx&vbI8}IioYX(2_hkqNn%SVMBJFL#_d1lYw4CXa1quV{>H%0tTtZoYIP=!AckdWo z6ZO1$ZK400kM!c^YVjQ{%uRqLWG>U<;kQ!b4<8mFnWbEAiPyy<`h*~*nX%Lt8lPzs zXRp#8hXWpQN72{{-6@zmyA!p9PCIy2bQ+dQ#hFWyE!9Jn2)xK1vPl9S8rM+pdw5(} z_xfQ`D(sY_(^Mg;BQ;bSEt_+f8Yhv@bqGJHdo9j5P}l&+3gC#eT3R8?U(ol@QhDVf zLGGe(`Gc=2-&GaAzPM3>DqbB|WSZ0nW67d9k<%)r`$b2HR3_OxYuV_s2J4J@-&rM? zN4?>TVrRG7gh;OV!y=cf(_6&_?2+}QtLw#m+tZxd=8bwrxR=WZ&M_s{d)q`sagYJ5 zL=2WSL0@4*6twD=3^OCl1G6aRf>A%JXM^{_kQrW?FjO)+M1jBJw_^PRB6{TobV||h z4NDaxY?$ezrMYpiJ|5HRW(@Q~ui>#u|}!jqcCGo3dT+ z&+Rlh;U&|@Af1udXZh^?BxpI%ff^HNFIeDa+5Udqs*D{)2{azj0o$QmZY8sChE_6L zpKyVdrJWi!ZaB5!@lDwFO@-pY;sLWTNydp8~C9YuqWi5v5dn6#fE4t|;Z-q@QD=d_Sl*z#%W`Pb+48(*^y%p@8d zn`V~(C4F76r*>;X5cK-;9_@bBypvS~Q!m2-7;_VLB%zy2vPNLW5Nx&V z@j>+K^}d4cT-X&2!!E$k1oCbLN-=LsV~&1t=PDa6jyH>$sWBQ~*1fFKsLbq#nZ9P% zMB?Od!O9=c!N3A|tQ{4k!1g4V&;gT2V0FLc6_`RVj?-~?t6;@_~#ui4ym#xo{A#B>H$yqLhBuJQjrKRelPDVn7J z0RW%CtpET33w%vBOWrt*t`3MP{Ozg7X{^0{3wEgOe?`pv}S}Z=Q2&kx_2-v#y zg;r4kYeAHXLZxb}dv8K&Fj(Q^aOTcAzk43PbLWD4A>ax4E6oz7W_`%zfe;t(7Hqsj z6gWE|9b66%)bU~#n8LihznW8jZ1jf3)m^xr#tFHaIC7 z<^CaO&jw_M6GnxN1;pfWv4EWLa&j67n@?ot;RAM%I2}ByGm!_)L)=b}%gT@Oq7NIaVyr$TnH~R1+9)6o z#Dx`~#`{xp^SPXh6Jh%OE%2d&;ohfoAtx8^o^G>drB5@CHmAdKBp4HoN&XIscEJX2 zVhQG0Q;aDwZcM)t?^TTPv9bM1O#h0>Z1O8{{YqkjUrF>Safz{h#q3x5Uzy?)`?p9) z@~s$Sj7W)1N{TW0m3W_$m>7eUxLC8v>{q;d#TiXWX20THi8Uw2_HO~(?N`ix#s4bK z(pLaauHDJm(mftn2UCW}o}KPn8J%m7Hpax5tnoH;v^B$o_;=i*2MrA@Es(+9*&%%|_fp(Grw{^K8bv&*>D{21=fG3L} z^i7!9c%IKrpOli61V+B>;%$)3)@(M#>6vQh+`<0JAR8c`{MM8-&SA5>HgQTCP$vwU z3tR;F4-d-B-#oLQU; zV6i}*IGCZ-#R+!QfG{ot1`LPKmJTDz0R%J$Qo%8mv)kQ3ZF1QL*#)iwDWAhx-Mj-x zIoM)YZg@y=LN=F?B>;)_pB^3>%6#rDJf>vXZj?fv039-&PSM+Z8L-6tlgy%Y)E{Bl8*<6AHy^;45B;77Jvf(^?T#hlm zgGh#szW;d7KQh+~B4)W@_p$)xJN=}h%bg>LS*jw28AE^bF{uJLDWQQpOrcUUfS{zZ zkkD{tM8rryV+u@%u}|nRi=ub;0mIQHmzp&VNRTA+4*f0k4Z%sxZ_Xja5Uo_E3-*4t zdrpmE-Ql$-B5#zHoQOQ$PT(YY^_1*dd1tx$P%jiUNZc-MEmt$I(Pd>faFvR|hmq_^ zANbSHjCvMspo)wd{^W>1NM%oj{!yva|LHlnPXsMdEZ9G9t; zmw_M%jKDB#$;E5Unx#lE@6zAzgQ)VrFd2Ddyg30OCPKuKYC1$Qdr|LBO4rnLPRXvl zP=e{>EGzD$wC!wt1#fq^K?f;kn0Ol*^7QxLUI1=e>YOP!sa<)?B&_R^nbBP zk!xc)&b&fjJrd+RRw}dj%R?#lT1M5ORj{=e?E051Lz))sHmrxG&M(OSH0u65 zC~D^tObUJd_o0a3N-OFh)X)2`G;4a0kVqg(%UOsLI-qk*XrwWIcnYfWI7kCnIcnOS5Lzk+x<{ z$$eSJQ_So1ji0@c5OJa=5Hcq9)k#_iIoY34xZ*&``Tdk`!-5lPc2&pwix#RXh z!@Tb@i;w-Jf$3n1y~{S9C3U+C&S7k21F3)0BCpzVzK+B& zCWMp`%qV)vBP~Ole3t5vP+Y2DCeTZLrnq9xj+J*v-LYc}DE7=ZA^J4SbKS@^9L^U` zRI>EE&pZJYh|6|$nO>6h?hEwE8dNUT6@Wy&!6+YKvq z4^8LWzcB|zJNNj_T1t1I;qVYY3G)hTs|~CBi2PM28nt^-$MIRG<%=qhHEIjm;1l^p z_pIwosk-vVg6qtxE+|?rSFa;(fBnBkZP_4e?*k8Rpo3-_GNS<@|>Xhu# zjVk>+7G-(is(n<%(4yMjdAhC|D41vHH4mIrkP`)~4_zd6CyuYe*`*)o-?d0qErdQa zG+aB-@S}$f>yaVLJWH<|X!!9%hM*f6?!(QCa2RvR<6Xz^ z_ASV%KEUbVBIxwP)t-5}%4<;6<4h*q_{;nkf>|;;D)n!OC4cvGkXBsWTv0>mN(%~y zu(gYeF#UU$QC^zhc*m?WH{?H}dH)rO=@+3WK_W_Pl-^x@tyIapO&9iV8DyWxsP_l5 zkql-dvkYDiK1J-Cjf7WgK-|>eOd{QWpg2b*N%x+&jAVrOQv>706>C@Y!1Rmj;UTfC zcP^&?z!Fk^u-Z_!{QFYn)8pW`ssnk-y(jg9`+zY%HO2p>LbI`c@t%W}?$-U*A?&5S zoc9zsQbtis9DN0$koYh*gCV)p5mvZ2p_mhNK!ucFSaVMK;?r_vgdBQ( z(G=u`59U=>uDf4ia5RoJReFL0w&x}&8A-qA2celUu+x?Sk}XI`bd9}&^$sNn&H%JkN} zC8$!qn(N4f(NrX3qw6kJ@5dg#M~u}5T#YbA=dBgHDBZpD`6_ls-3$7umgwU(_u4em zH_cy){kjo~+uLiu(h$w(j>D4-eV7`N_zpdyn=1UoH?H5*fIap@fGIWveF)U(Lm-7P z1Vs?b)l1aW5jfYA=p$al-=p{7coAqwc79oIsD^)v`8$by@Oqd`&+ok0^mw>^LDvsi z8-$~*C5)C zm8zcCEP3@(r3zNaBjFP;|GhwJ7!4#1_+ZpmnHF9}qrw}$g9W5jJVE{K8 zcT!K5GTGJKy1Syt_aD!H8o9lPPg>#P$|yQ5eF30togM!zAv702?n~$9&sdKwpP6n} tv8Jc9shK<3|DG*O?mlS6yfQcR_Tu5!ZwLDL^5~a#I3J3iK#xRSf@Rp1DRTe- literal 0 HcmV?d00001 From 0909bda7dc9e8df32d9f8a744231cd3cf47e9cba Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Fri, 31 Mar 2017 11:11:44 +0200 Subject: [PATCH 02/14] Refactored turn tests a little Moved restrictTurn to own class so that it can be used in multiple tests --- .../r5/streets/TurnCostCalculatorTest.java | 1 + .../r5/streets/TurnRestrictionTest.java | 5 +++++ .../com/conveyal/r5/streets/TurnTest.java | 14 +------------ .../conveyal/r5/streets/TurnTestUtils.java | 20 +++++++++++++++++++ 4 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/conveyal/r5/streets/TurnTestUtils.java diff --git a/src/test/java/com/conveyal/r5/streets/TurnCostCalculatorTest.java b/src/test/java/com/conveyal/r5/streets/TurnCostCalculatorTest.java index bb7e442b3..e315659f2 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnCostCalculatorTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnCostCalculatorTest.java @@ -6,6 +6,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * Created by matthewc on 2/23/16. diff --git a/src/test/java/com/conveyal/r5/streets/TurnRestrictionTest.java b/src/test/java/com/conveyal/r5/streets/TurnRestrictionTest.java index b0869144c..1ac2c9ce2 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnRestrictionTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnRestrictionTest.java @@ -3,6 +3,11 @@ import com.conveyal.r5.profile.StreetMode; import org.junit.Test; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertEquals; + public class TurnRestrictionTest extends TurnTest { @Test public void testSimpleNoTurn () { diff --git a/src/test/java/com/conveyal/r5/streets/TurnTest.java b/src/test/java/com/conveyal/r5/streets/TurnTest.java index c434aac15..77fe5cf0a 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnTest.java @@ -6,8 +6,7 @@ /** * Base class for tests of turn costs and restrictions.. */ -public abstract class TurnTest extends TestCase { - public StreetLayer streetLayer; +public abstract class TurnTest extends TurnTestUtils { // center vertex index, n/s/e/w vertex indices, n/s/e/w edge indices (always starting from center). public int VCENTER, VN, VS, VE, VW, VNE, VNW, EN, ES, EE, EW, ENE, ENW; @@ -51,15 +50,4 @@ public void setUp (boolean southernHemisphere) { streetLayer.buildEdgeLists(); } - /** create a turn restriction */ - public void restrictTurn (boolean onlyTurn, int from, int to, int... via) { - TurnRestriction restriction = new TurnRestriction(); - restriction.fromEdge = from; - restriction.toEdge = to; - restriction.only = onlyTurn; - restriction.viaEdges = via; - int ridx = streetLayer.turnRestrictions.size(); - streetLayer.turnRestrictions.add(restriction); - streetLayer.edgeStore.turnRestrictions.put(restriction.fromEdge, ridx); - } } diff --git a/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java b/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java new file mode 100644 index 000000000..bff16d80a --- /dev/null +++ b/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java @@ -0,0 +1,20 @@ +package com.conveyal.r5.streets; + +/** + * Moved from TurnTest + */ +public class TurnTestUtils { + public StreetLayer streetLayer; + + /** create a turn restriction */ + public void restrictTurn (boolean onlyTurn, int from, int to, int... via) { + TurnRestriction restriction = new TurnRestriction(); + restriction.fromEdge = from; + restriction.toEdge = to; + restriction.only = onlyTurn; + restriction.viaEdges = via; + int ridx = streetLayer.turnRestrictions.size(); + streetLayer.turnRestrictions.add(restriction); + streetLayer.edgeStore.turnRestrictions.put(restriction.fromEdge, ridx); + } +} From 5855e029f4f950fd77b0f20256c22b927016257b Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Fri, 31 Mar 2017 11:12:58 +0200 Subject: [PATCH 03/14] Add turn split test on to edge --- .../conveyal/r5/streets/TurnSplitTest.java | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index e8dc3db52..384f25cb8 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -116,4 +116,86 @@ public void testTurnRestrictionWithSplit() throws Exception { } } + + //Test if turn restriction still works if toEdge is split + //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops + @Test + public void testTurnRestrictionWithSplit1() throws Exception { + + ProfileRequest profileRequest = new ProfileRequest(); + profileRequest.fromLat = 38.8930088; + profileRequest.fromLon = -76.998343; + profileRequest.toLat = 38.8914185; + profileRequest.toLon = -76.9962294; + + StreetRouter streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + StreetRouter.State lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + + int oldWeight = lastState.weight; + StreetRouter.State state = lastState; + EdgeStore.Edge edge = sl.edgeStore.getCursor(); + + TurnRestriction tr = sl.turnRestrictions.get(1); + + LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getFlagsAsString()); + //Turn from 143 to 145 is in Turn restriction + assertFalse(state.backEdge == 134 && (state.backState.backEdge == 152 || state.backState.backEdge == 146)); + state = state.backState; + + } + + //Splits "to Turn restriction edge" + int vertex = sl.createAndLinkVertex(38.8919775,-76.996019); + + sl.buildEdgeLists(); + + streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + state = lastState; + + int newWeight = lastState.weight; + + assertEquals(oldWeight, newWeight); + + + //LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.info("V:{} W:{} be:{}, flags:{}", state.vertex, state.weight, state.backEdge, edge.getFlagsAsString()); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getFlagsAsString()); + //Turn from 143 to 146 is in Turn restriction + assertFalse(state.backEdge == 134 && (state.backState.backEdge == 152 || state.backState.backEdge == 146)); + state = state.backState; + + } + + } } From 4fddb078b787d7b8ebfba2194d76507f76c5a063 Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Mon, 3 Apr 2017 12:43:45 +0200 Subject: [PATCH 04/14] Added support for splitting via turn restriction edges Previously when via Turn restriction edges were split. Turn restrictions didn't apply anymore since via edges weren't complete. Since splitting creates new edges which wasn't in turn restrictions. Commit fixes that and also adds tests. --- .../com/conveyal/r5/streets/EdgeStore.java | 7 ++ .../com/conveyal/r5/streets/StreetLayer.java | 39 +++++++++ .../conveyal/r5/streets/TurnSplitTest.java | 87 +++++++++++++++++++ 3 files changed, 133 insertions(+) diff --git a/src/main/java/com/conveyal/r5/streets/EdgeStore.java b/src/main/java/com/conveyal/r5/streets/EdgeStore.java index f74229090..b5befb4bc 100644 --- a/src/main/java/com/conveyal/r5/streets/EdgeStore.java +++ b/src/main/java/com/conveyal/r5/streets/EdgeStore.java @@ -142,6 +142,11 @@ public boolean isExtendOnlyCopy() { /** Turn restrictions for turning _out of_ each edge */ public TIntIntMultimap turnRestrictions; + /** Via edges to turn restrictions link. Used when splitting edges + * FIXME: Can be probably transient since it's used only when building graph in getOrCreateVertexNear + * But I'm not sure what is with scenarios (materializeOne function)*/ + public TIntIntMultimap turnRestrictionsVia; + public StreetLayer layer; public static final transient EnumSet PERMISSION_FLAGS = EnumSet @@ -166,6 +171,7 @@ public EdgeStore (VertexStore vertexStore, StreetLayer layer, int initialSize) { inAngles = new TByteArrayList(initialEdgePairs); outAngles = new TByteArrayList(initialEdgePairs); turnRestrictions = new TIntIntHashMultimap(); + turnRestrictionsVia = new TIntIntHashMultimap(); } /** @@ -1007,6 +1013,7 @@ public EdgeStore extendOnlyCopy() { copy.outAngles = new TByteArrayList(outAngles); // We don't expect to add/change any turn restrictions. copy.turnRestrictions = turnRestrictions; + copy.turnRestrictionsVia = turnRestrictionsVia; return copy; } diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 1c054706d..991cb2475 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -31,6 +31,7 @@ import gnu.trove.map.hash.TLongIntHashMap; import gnu.trove.set.TIntSet; import gnu.trove.set.hash.TIntHashSet; +import org.apache.commons.lang.ArrayUtils; import org.geotools.geojson.geom.GeometryJSON; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -679,6 +680,9 @@ else if ("via".equals(member.role)) { int index = turnRestrictions.size(); turnRestrictions.add(out); edgeStore.turnRestrictions.put(out.fromEdge, index); + for(int edgeId: out.viaEdges) { + edgeStore.turnRestrictionsVia.put(edgeId, index); + } } else { // via member(s) are ways, which is more tricky // do a little street search constrained to the ways in question @@ -869,6 +873,9 @@ else if ("via".equals(member.role)) { int index = turnRestrictions.size(); turnRestrictions.add(out); edgeStore.turnRestrictions.put(out.fromEdge, index); + for(int edgeId: out.viaEdges) { + edgeStore.turnRestrictionsVia.put(edgeId, index); + } // take a deep breath } @@ -1145,6 +1152,22 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) // where they are edgeStore.turnRestrictions.removeAll(split.edge).forEach(ridx -> edgeStore.turnRestrictions.put(newEdge1.edgeIndex, ridx)); + //Turn restrictions that are on via edge should be updated. Since we now have 2 via edges from one, because we split one + if (edgeStore.turnRestrictionsVia.containsKey(split.edge)) { + + edgeStore.turnRestrictionsVia.get(split.edge).forEach(turnRestrictionIndex -> { + TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); + int currentViaEdgeIndex = ArrayUtils.indexOf(tr.viaEdges, split.edge); + //New via edge index is added as the next edge in viaEdges list + //noinspection UnnecessaryLocalVariable + int[] newViaEdges = ArrayUtils.add(tr.viaEdges, currentViaEdgeIndex+1, newEdge1.edgeIndex); + tr.viaEdges = newViaEdges; + //Also updates turnRestrictionsViaList + edgeStore.turnRestrictionsVia.put(newEdge1.edgeIndex, turnRestrictionIndex); + return true; + }); + } + // Insert the new edge into the spatial index if (!edgeStore.isExtendOnlyCopy()) { spatialIndex.insert(newEdge1.getEnvelope(), newEdge1.edgeIndex); @@ -1199,6 +1222,22 @@ public int splitEdge(Split split) { // where they are edgeStore.turnRestrictions.removeAll(split.edge).forEach(ridx -> edgeStore.turnRestrictions.put(newEdge.edgeIndex, ridx)); + //Turn restrictions that are on via edge should be updated. Since we now have 2 via edges from one, because we split one + if (edgeStore.turnRestrictionsVia.containsKey(split.edge)) { + + edgeStore.turnRestrictionsVia.get(split.edge).forEach(turnRestrictionIndex -> { + TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); + int currentViaEdgeIndex = ArrayUtils.indexOf(tr.viaEdges, split.edge); + //New via edge index is added as the next edge in viaEdges list + //noinspection UnnecessaryLocalVariable + int[] newViaEdges = ArrayUtils.add(tr.viaEdges, currentViaEdgeIndex+1, newEdge.edgeIndex); + tr.viaEdges = newViaEdges; + //Also updates turnRestrictionsViaList + edgeStore.turnRestrictionsVia.put(newEdge.edgeIndex, turnRestrictionIndex); + return true; + }); + } + return newVertexIndex; // TODO store street-to-stop distance in a table in TransitLayer. This also allows adjusting for subway entrances etc. } diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index 384f25cb8..659ddb714 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -198,4 +198,91 @@ public void testTurnRestrictionWithSplit1() throws Exception { } } + + + //Test if turn restriction still works if viaEdge is split + //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops + @Test + public void testTurnRestrictionWithSplit2() throws Exception { + + ProfileRequest profileRequest = new ProfileRequest(); + profileRequest.fromLat = 38.8930088; + profileRequest.fromLon = -76.998343; + profileRequest.toLat = 38.8914185; + profileRequest.toLon = -76.99933; + + //Changes turnRestriction from:146 to 134 to 146 via 134 to 136 + sl.turnRestrictions.get(1).toEdge = 136; + sl.turnRestrictions.get(1).viaEdges = new int[]{ 134 }; + sl.edgeStore.turnRestrictionsVia.put(134, 1); + + StreetRouter streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + StreetRouter.State lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + + int oldWeight = lastState.weight; + StreetRouter.State state = lastState; + EdgeStore.Edge edge = sl.edgeStore.getCursor(); + + TurnRestriction tr = sl.turnRestrictions.get(1); + + LOG.debug("TR:{}->{}->{}", tr.fromEdge, tr.viaEdges, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getFlagsAsString()); + //Turn from 143 to 145 is in Turn restriction + assertFalse(state.backEdge == 134 && (state.backState.backEdge == 152 || state.backState.backEdge == 146)); + state = state.backState; + + } + + //Splits "via Turn restriction edge 134" + int vertex = sl.createAndLinkVertex(38.8919775,-76.996019); + + sl.buildEdgeLists(); + + streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + state = lastState; + + int newWeight = lastState.weight; + + + //LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getFlagsAsString()); + //Turn from 143 to 146 is in Turn restriction + assertFalse(state.backEdge == 134 && (state.backState.backEdge == 152 || state.backState.backEdge == 146)); + state = state.backState; + + } + + assertEquals(oldWeight, newWeight); + + } } From ba75d24851d20bd7c16d1329a477e196a0ee7df5 Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Tue, 11 Apr 2017 12:02:45 +0200 Subject: [PATCH 05/14] RestrictTurn needs to add reverse restriction too --- src/test/java/com/conveyal/r5/streets/TurnTestUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java b/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java index bff16d80a..3e74294f5 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java +++ b/src/test/java/com/conveyal/r5/streets/TurnTestUtils.java @@ -16,5 +16,6 @@ public void restrictTurn (boolean onlyTurn, int from, int to, int... via) { int ridx = streetLayer.turnRestrictions.size(); streetLayer.turnRestrictions.add(restriction); streetLayer.edgeStore.turnRestrictions.put(restriction.fromEdge, ridx); + streetLayer.addReverseTurnRestriction(restriction, ridx); } } From 3014f63b439607e9aa0c35814d7f3b7b9805e74e Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Tue, 11 Apr 2017 11:18:48 +0200 Subject: [PATCH 06/14] Removing island can be enabled/disabled with textConfig This is also needed for turn restriction tests. Default is true. As it was now. So nothing should change. --- .../builder/TNBuilderConfig.java | 30 +++++++++++++++++++ .../com/conveyal/r5/streets/StreetLayer.java | 2 +- .../conveyal/r5/transit/TransportNetwork.java | 2 +- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/point_to_point/builder/TNBuilderConfig.java b/src/main/java/com/conveyal/r5/point_to_point/builder/TNBuilderConfig.java index db47852be..9abc5f3ac 100644 --- a/src/main/java/com/conveyal/r5/point_to_point/builder/TNBuilderConfig.java +++ b/src/main/java/com/conveyal/r5/point_to_point/builder/TNBuilderConfig.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; +import com.google.common.annotations.VisibleForTesting; import java.io.File; @@ -84,6 +85,11 @@ public class TNBuilderConfig { */ public final boolean fetchElevationUS; + /** + * If non accessible islands should be removed (default true) + */ + public final boolean removeIslands; + /** If specified, download NED elevation tiles from the given AWS S3 bucket. */ //public final S3BucketConfig elevationBucket; @@ -144,6 +150,30 @@ public TNBuilderConfig() { bikeRentalFile = null; speeds = SpeedConfig.defaultConfig(); analysisFareCalculator = null; + removeIslands = true; + } + + @VisibleForTesting + public TNBuilderConfig(boolean removeIslands) { + htmlAnnotations = false; + maxHtmlAnnotationsPerFile = 1000; + transit = true; + useTransfersTxt = false; + parentStopLinking = false; + stationTransfers = false; + subwayAccessTime = DEFAULT_SUBWAY_ACCESS_TIME; + streets = true; + embedRouterConfig = true; + areaVisibility = false; + matchBusRoutesToStreets = false; + fetchElevationUS = false; + staticBikeRental = false; + staticParkAndRide = true; + staticBikeParkAndRide = false; + bikeRentalFile = null; + speeds = SpeedConfig.defaultConfig(); + analysisFareCalculator = null; + this.removeIslands = removeIslands; } public static TNBuilderConfig defaultConfig() { diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 20e363a38..41d1d9d6f 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -238,7 +238,7 @@ private static boolean actuallyExistsInReality(String highway, Way way) { /** Load OSM, optionally removing floating subgraphs (recommended) */ - void loadFromOsm (OSM osm, boolean removeIslands, boolean saveVertexIndex) { + public void loadFromOsm(OSM osm, boolean removeIslands, boolean saveVertexIndex) { if (!osm.intersectionDetection) throw new IllegalArgumentException("Intersection detection not enabled on OSM source"); diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java index b0239c037..f156101cc 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java @@ -163,7 +163,7 @@ private static TransportNetwork fromFiles (String osmSourceFile, List gt StreetLayer streetLayer = new StreetLayer(tnBuilderConfig); transportNetwork.streetLayer = streetLayer; streetLayer.parentNetwork = transportNetwork; - streetLayer.loadFromOsm(osm); + streetLayer.loadFromOsm(osm, tnBuilderConfig.removeIslands, false); osm.close(); // The street index is needed for associating transit stops with the street network From 72b60fd0d157d0b7d45b3aceb9d1d2128b71effe Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Tue, 11 Apr 2017 12:14:14 +0200 Subject: [PATCH 07/14] Turn restriction are correctly updated in scenario If adding stops in scenario splits from/to/via edges Turn restrictions need to be updated in new transportNetwor. But they need to stay the same in original. For this we copy all 4 turn restriction fields. And make copy of each TurnRestriction which needs new from/to/via edges in this copied list. We could copy on write since chance that turn restrictions needs to be copied is small, but Turn Restrictions are small and if need arises that COW is needed we would do it. Tests are also created --- .../com/conveyal/r5/streets/EdgeStore.java | 8 +- .../com/conveyal/r5/streets/StreetLayer.java | 61 ++++-- .../conveyal/r5/streets/TurnRestriction.java | 14 ++ .../conveyal/r5/util/TIntIntHashMultimap.java | 9 + .../r5/analyst/scenario/FakeGraph.java | 36 ++++ .../ScenarioModifyTurnRestrictions.java | 181 ++++++++++++++++++ 6 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 src/test/java/com/conveyal/r5/streets/ScenarioModifyTurnRestrictions.java diff --git a/src/main/java/com/conveyal/r5/streets/EdgeStore.java b/src/main/java/com/conveyal/r5/streets/EdgeStore.java index 791c85b89..3f83a5fa3 100644 --- a/src/main/java/com/conveyal/r5/streets/EdgeStore.java +++ b/src/main/java/com/conveyal/r5/streets/EdgeStore.java @@ -1068,9 +1068,11 @@ public EdgeStore extendOnlyCopy() { copy.inAngles = new TByteArrayList(inAngles); copy.outAngles = new TByteArrayList(outAngles); // We don't expect to add/change any turn restrictions. - copy.turnRestrictions = turnRestrictions; - copy.turnRestrictionsReverse = turnRestrictionsReverse; - copy.turnRestrictionsVia = turnRestrictionsVia; + copy.turnRestrictions = new TIntIntHashMultimap((TIntIntHashMultimap) turnRestrictions); + copy.turnRestrictionsReverse = new TIntIntHashMultimap( + (TIntIntHashMultimap) turnRestrictionsReverse); + copy.turnRestrictionsVia = new TIntIntHashMultimap( + (TIntIntHashMultimap) turnRestrictionsVia); return copy; } diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 41d1d9d6f..25ff9812e 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -1183,23 +1183,51 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) // clean up any turn restrictions that exist // turn restrictions on the forward edge go to the new edge's forward edge. Turn restrictions on the back edge stay // where they are - edgeStore.turnRestrictions.removeAll(split.edge).forEach(ridx -> edgeStore.turnRestrictions.put(newEdge1.edgeIndex, ridx)); + edgeStore.turnRestrictions.removeAll(split.edge).forEach(ridx -> { + TurnRestriction tr = turnRestrictions.get(ridx); + TurnRestriction newTurnRestriction; + // If edge is mutable we are splitting normal edge and can modify existing turnRestriction with new fromEdge + if (edge.isMutable()) { + newTurnRestriction = tr; + } else { + //If it isn't we are in scenario. Which means we need to replace current turnRestriction with new one + //Since TurnRestriction in original graph needs to stay the same + newTurnRestriction = new TurnRestriction(tr); + } + newTurnRestriction.fromEdge = newEdge1.edgeIndex; + + //In scenario we replace existing turnRestriction with new one + if (!edge.isMutable()) { + turnRestrictions.remove(ridx); + turnRestrictions.add(ridx, newTurnRestriction); + } + edgeStore.turnRestrictions.put(newEdge1.edgeIndex, ridx); + return true; + }); //Turn restrictions that are on via edge should be updated. Since we now have 2 via edges from one, because we split one - if (edgeStore.turnRestrictionsVia.containsKey(split.edge)) { + edgeStore.turnRestrictionsVia.get(split.edge).forEach(turnRestrictionIndex -> { + TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); + TurnRestriction newTurnRestriction; + int currentViaEdgeIndex = ArrayUtils.indexOf(tr.viaEdges, split.edge); + //New via edge index is added as the next edge in viaEdges list + //noinspection UnnecessaryLocalVariable + int[] newViaEdges = ArrayUtils.add(tr.viaEdges, currentViaEdgeIndex+1, newEdge1.edgeIndex); + if (edge.isMutable()) { + newTurnRestriction = tr; + } else { + newTurnRestriction = new TurnRestriction(tr); + } + newTurnRestriction.viaEdges = newViaEdges; + if (!edge.isMutable()) { + turnRestrictions.remove(turnRestrictionIndex); + turnRestrictions.add(turnRestrictionIndex, newTurnRestriction); + } + //Also updates turnRestrictionsViaList + edgeStore.turnRestrictionsVia.put(newEdge1.edgeIndex, turnRestrictionIndex); + return true; + }); - edgeStore.turnRestrictionsVia.get(split.edge).forEach(turnRestrictionIndex -> { - TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); - int currentViaEdgeIndex = ArrayUtils.indexOf(tr.viaEdges, split.edge); - //New via edge index is added as the next edge in viaEdges list - //noinspection UnnecessaryLocalVariable - int[] newViaEdges = ArrayUtils.add(tr.viaEdges, currentViaEdgeIndex+1, newEdge1.edgeIndex); - tr.viaEdges = newViaEdges; - //Also updates turnRestrictionsViaList - edgeStore.turnRestrictionsVia.put(newEdge1.edgeIndex, turnRestrictionIndex); - return true; - }); - } // Insert the new edge into the spatial index if (!edgeStore.isExtendOnlyCopy()) { @@ -1384,6 +1412,11 @@ public StreetLayer scenarioCopy(TransportNetwork newScenarioNetwork, boolean wil // The extend-only copy of the EdgeStore also contains a new extend-only copy of the VertexStore. copy.vertexStore = copy.edgeStore.vertexStore; copy.temporaryEdgeIndex = new IntHashGrid(); + // List of turn restrictions needs to be copied because if we split streets + // (when adding transit stops for example) that are part of turn restrictions we need to + // update TurnRestriction from/to/via edges + copy.turnRestrictions = new ArrayList<>(turnRestrictions.size()); + copy.turnRestrictions.addAll(turnRestrictions); } copy.parentNetwork = newScenarioNetwork; copy.baseStreetLayer = this; diff --git a/src/main/java/com/conveyal/r5/streets/TurnRestriction.java b/src/main/java/com/conveyal/r5/streets/TurnRestriction.java index 80a553df3..171277ed8 100644 --- a/src/main/java/com/conveyal/r5/streets/TurnRestriction.java +++ b/src/main/java/com/conveyal/r5/streets/TurnRestriction.java @@ -30,6 +30,20 @@ public class TurnRestriction implements Serializable { /** the intermediate edges in this turn restriction */ public int[] viaEdges = EMPTY_INT_ARRAY; + // Copy constructor, used in scenarios, since street splitting can change turn restriction edges + public TurnRestriction(TurnRestriction tr) { + this.only = tr.only; + this.fromEdge = tr.fromEdge; + this.toEdge = tr.toEdge; + if (tr.viaEdges != EMPTY_INT_ARRAY) { + this.viaEdges = Arrays.copyOf(tr.viaEdges, tr.viaEdges.length); + } + } + + public TurnRestriction() { + + } + /** * Reverses order of viaEdges this is used in reverse streetSearch for turn restrictions * diff --git a/src/main/java/com/conveyal/r5/util/TIntIntHashMultimap.java b/src/main/java/com/conveyal/r5/util/TIntIntHashMultimap.java index 212158d3c..afe470f6f 100644 --- a/src/main/java/com/conveyal/r5/util/TIntIntHashMultimap.java +++ b/src/main/java/com/conveyal/r5/util/TIntIntHashMultimap.java @@ -20,6 +20,15 @@ public class TIntIntHashMultimap implements TIntIntMultimap, Serializable { private TIntObjectMap wrapped = new TIntObjectHashMap<>(); + //Copy constructor + public TIntIntHashMultimap (TIntIntHashMultimap base) { + wrapped.putAll(base.wrapped); + } + + public TIntIntHashMultimap() { + + } + @Override public boolean put(int key, int value) { if (!wrapped.containsKey(key)) wrapped.put(key, new TIntArrayList()); diff --git a/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java b/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java index 50452cfa5..63677f9ec 100644 --- a/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java +++ b/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java @@ -76,6 +76,42 @@ public static TransportNetwork buildNetwork (TransitNetwork... networks) { } } + /** Build a graph from provided OSM using requested transit feeds */ + public static TransportNetwork buildNetwork (InputStream osmFileStream, String osmFilename, + TNBuilderConfig tnBuilderConfig, TransitNetwork... networks) { + try { + File osmFile = File.createTempFile(osmFilename, ".osm.pbf"); + + InputStream is = new BufferedInputStream(osmFileStream); + OutputStream os = new BufferedOutputStream(new FileOutputStream(osmFile)); + ByteStreams.copy(is, os); + is.close(); + os.close(); + + List filesToDelete = new ArrayList<>(); + filesToDelete.add(osmFile); + + List gtfsFiles = new ArrayList<>(); + + for (TransitNetwork network : networks) { + File gtfsFile = File.createTempFile(network.toString(), ".gtfs.zip"); + network.get().toFile(gtfsFile.getAbsolutePath()); + filesToDelete.add(gtfsFile); + gtfsFiles.add(gtfsFile.getAbsolutePath()); + } + + TransportNetwork net = TransportNetwork.fromFiles(osmFile.getAbsolutePath(), gtfsFiles, tnBuilderConfig); + net.transitLayer.buildDistanceTables(null); + + // clean up + filesToDelete.forEach(f -> f.delete()); + + return net; + } catch (Exception e) { + throw new RuntimeException(e); + } + } + /** Add transit (not just stops) to a Columbus graph */ public static GTFSFeed getTransit () throws Exception { // using conveyal GTFS lib to build GTFS so a lot of code does not have to be rewritten later diff --git a/src/test/java/com/conveyal/r5/streets/ScenarioModifyTurnRestrictions.java b/src/test/java/com/conveyal/r5/streets/ScenarioModifyTurnRestrictions.java new file mode 100644 index 000000000..d439d0c25 --- /dev/null +++ b/src/test/java/com/conveyal/r5/streets/ScenarioModifyTurnRestrictions.java @@ -0,0 +1,181 @@ +package com.conveyal.r5.streets; + +import com.conveyal.gtfs.model.Route; +import com.conveyal.r5.analyst.scenario.AddTrips; +import com.conveyal.r5.analyst.scenario.FakeGraph; +import com.conveyal.r5.analyst.scenario.Scenario; +import com.conveyal.r5.analyst.scenario.StopSpec; +import com.conveyal.r5.point_to_point.builder.TNBuilderConfig; +import com.conveyal.r5.transit.TransportNetwork; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Arrays; + +import static com.conveyal.r5.analyst.scenario.FakeGraph.buildNetwork; +import static org.junit.Assert.*; + +/** + * Tests that turn restriction works after scenario splits from/to/via edges + * + * Works means that turn restriction on original network stay the same (turnRestriction field in edgeStore and streetLayer) and + * turnRestrictionsVia, turnRestrictionsReverse in Edgestore. + * + * And that in network with scenario they are updated. + * Created by mabu on 11.4.2017. + */ +public class ScenarioModifyTurnRestrictions { + private static final Logger LOG = LoggerFactory.getLogger(ScenarioModifyTurnRestrictions.class); + + private TransportNetwork network; + private long checksum; + + @Before + public void setUpGraph() throws Exception { + TNBuilderConfig tnBuilderConfig = new TNBuilderConfig(false); + network = buildNetwork(StreetLayerTest.class.getResourceAsStream("turn-restriction-split-test.pbf"), + "turn-restriction-split-test", tnBuilderConfig, + FakeGraph.TransitNetwork.SINGLE_LINE); + checksum = network.checksum(); + + } + + //Creates Scenario with trip on specified stop and applies it to network + //Returns modified network + private TransportNetwork addStop(double lon, double lat) { + AddTrips at = new AddTrips(); + at.bidirectional = true; + at.stops = Arrays.asList( + new StopSpec(lon, lat), + new StopSpec(-83.0014, 39.962), + new StopSpec(-82.9495, 39.962) + ); + at.mode = Route.BUS; + + AddTrips.PatternTimetable entry = new AddTrips.PatternTimetable(); + entry.headwaySecs = 900; + entry.monday = entry.tuesday = entry.wednesday = entry.thursday = entry.friday = true; + entry.saturday = entry.sunday = false; + entry.hopTimes = new int[] { 120, 140 }; + entry.dwellTimes = new int[] { 0, 30, 0 }; + entry.startTime = 7 * 3600; + entry.endTime = 10 * 3600; + + at.frequencies = Arrays.asList(entry); + + Scenario scenario = new Scenario(); + scenario.modifications = Arrays.asList(at); + + return scenario.applyToTransportNetwork(network); + } + + @Test + public void testStopSplitsFromEdge() throws Exception { + TurnRestriction tr = network.streetLayer.turnRestrictions.get(1); + LOG.debug("TR:{}", tr); + int turnRestrictionFrom = tr.fromEdge; + int turnRestrictionSize = network.streetLayer.turnRestrictions.size(); + + int[] restrictionsOnFrom = network.streetLayer.edgeStore.turnRestrictions + .get(turnRestrictionFrom).toArray(); + + //Stop was added that splits fromEdge on turn Restriction + TransportNetwork mod = addStop(-76.9959183, 38.8921836); + + //Size of turn restrictions needs to be the same + + int copyTurnRestrictionSize = mod.streetLayer.turnRestrictions.size(); + + //List of turn restrictions needs to be copied since if we add transitStops that split turnRestriction edges + //We need to update turnRestrictions (since edges are different) and this needs to be different than original TurnRestriction list + assertEquals(turnRestrictionSize, copyTurnRestrictionSize); + + //Original turnRestriction needs to remain the same + assertEquals(turnRestrictionFrom, network.streetLayer.turnRestrictions.get(1).fromEdge); + + assertArrayEquals(restrictionsOnFrom, network.streetLayer.edgeStore.turnRestrictions.get(turnRestrictionFrom).toArray()); + + //copied streetLayer needs to have new from edge + assertNotEquals(turnRestrictionFrom, mod.streetLayer.turnRestrictions.get(1).fromEdge); + + assertNotEquals(restrictionsOnFrom, mod.streetLayer.edgeStore.turnRestrictions.get(turnRestrictionFrom).toArray()); + } + + @Test + public void testStopSplitsToEdge() throws Exception { + TurnRestriction tr = network.streetLayer.turnRestrictions.get(1); + LOG.info("TR:{}", tr); + int turnRestrictionTo = tr.toEdge; + int turnRestrictionSize = network.streetLayer.turnRestrictions.size(); + + int[] restrictionsOnTo = network.streetLayer.edgeStore.turnRestrictionsReverse + .get(turnRestrictionTo).toArray(); + + //Stop was added that splits toEdge on turn Restriction + TransportNetwork mod = addStop(-76.996019,38.8919775); + + //Size of turn restrictions needs to be the same + + int copyTurnRestrictionSize = mod.streetLayer.turnRestrictions.size(); + + //List of turn restrictions needs to be copied since if we add transitStops that split turnRestriction edges + //We need to update turnRestrictions (since edges are different) and this needs to be different than original TurnRestriction list + assertEquals(turnRestrictionSize, copyTurnRestrictionSize); + + //Original turnRestriction needs to remain the same + assertEquals(turnRestrictionTo, network.streetLayer.turnRestrictions.get(1).toEdge); + + assertArrayEquals(restrictionsOnTo, network.streetLayer.edgeStore.turnRestrictionsReverse.get(turnRestrictionTo).toArray()); + + //copied streetLayer needs to have same to edge + assertEquals(turnRestrictionTo, mod.streetLayer.turnRestrictions.get(1).toEdge); + + assertArrayEquals(restrictionsOnTo, mod.streetLayer.edgeStore.turnRestrictionsReverse.get(turnRestrictionTo).toArray()); + } + + @Test + public void testStopSplitsViaEdge() throws Exception { + + //Changes turnRestriction from:146 to 134 to 146 via 134 to 136 + network.streetLayer.turnRestrictions.get(1).toEdge = 136; + network.streetLayer.turnRestrictions.get(1).viaEdges = new int[]{ 134 }; + network.streetLayer.edgeStore.turnRestrictionsVia.put(134, 1); + + TurnRestriction tr = network.streetLayer.turnRestrictions.get(1); + LOG.info("TR:{}", tr); + int turnRestrictionFrom = tr.fromEdge; + int[] turnRestrictionsVia = tr.viaEdges; + int turnRestrictionSize = network.streetLayer.turnRestrictions.size(); + + int[] restrictionsOnVia = network.streetLayer.edgeStore.turnRestrictionsVia + .get(turnRestrictionsVia[0]).toArray(); + + //Stop was added that splits viaEdge on turn Restriction + TransportNetwork mod = addStop(-76.996019, 38.8919775); + + //Size of turn restrictions needs to be the same + + int copyTurnRestrictionSize = mod.streetLayer.turnRestrictions.size(); + + //List of turn restrictions needs to be copied since if we add transitStops that split turnRestriction edges + //We need to update turnRestrictions (since edges are different) and this needs to be different than original TurnRestriction list + assertEquals(turnRestrictionSize, copyTurnRestrictionSize); + + //Original turnRestriction needs to remain the same + assertEquals(turnRestrictionFrom, network.streetLayer.turnRestrictions.get(1).fromEdge); + + assertArrayEquals(turnRestrictionsVia, network.streetLayer.turnRestrictions.get(1).viaEdges); + + assertArrayEquals(restrictionsOnVia, network.streetLayer.edgeStore.turnRestrictionsVia.get(turnRestrictionsVia[0]).toArray()); + assertFalse(network.streetLayer.edgeStore.turnRestrictionsVia.containsKey(154)); + + //copied streetLayer needs to have new via edges + assertEquals(turnRestrictionFrom, mod.streetLayer.turnRestrictions.get(1).fromEdge); + + assertEquals(turnRestrictionsVia.length+1, mod.streetLayer.turnRestrictions.get(1).viaEdges.length); + + assertArrayEquals(new int[]{1}, mod.streetLayer.edgeStore.turnRestrictionsVia.get(154).toArray()); + } +} From 4ee887ed49c44600f1824b9587c837aff3ad76aa Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Thu, 13 Apr 2017 09:22:41 +0200 Subject: [PATCH 08/14] Renamed turnSplitTests to be more descriptive --- src/test/java/com/conveyal/r5/streets/TurnSplitTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index 659ddb714..2a2e05af7 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -38,7 +38,7 @@ public void setUpGraph() throws Exception { //Test if turn restriction still works if fromEdge is split //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops @Test - public void testTurnRestrictionWithSplit() throws Exception { + public void testTurnRestrictionWithSplitOnFrom() throws Exception { ProfileRequest profileRequest = new ProfileRequest(); profileRequest.fromLat = 38.8930088; @@ -120,7 +120,7 @@ public void testTurnRestrictionWithSplit() throws Exception { //Test if turn restriction still works if toEdge is split //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops @Test - public void testTurnRestrictionWithSplit1() throws Exception { + public void testTurnRestrictionWithSplitOnTo() throws Exception { ProfileRequest profileRequest = new ProfileRequest(); profileRequest.fromLat = 38.8930088; @@ -203,7 +203,7 @@ public void testTurnRestrictionWithSplit1() throws Exception { //Test if turn restriction still works if viaEdge is split //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops @Test - public void testTurnRestrictionWithSplit2() throws Exception { + public void testTurnRestrictionWithSplitOnVia() throws Exception { ProfileRequest profileRequest = new ProfileRequest(); profileRequest.fromLat = 38.8930088; From fedab58dbdab6dcc0d4f0cdc533f8b327f06cf5e Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Thu, 13 Apr 2017 11:04:10 +0200 Subject: [PATCH 09/14] Add test for spliting TR on backward edge --- .../conveyal/r5/streets/TurnSplitTest.java | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index 2a2e05af7..c08ca6bee 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -117,6 +117,100 @@ public void testTurnRestrictionWithSplitOnFrom() throws Exception { } + //Test if turn restriction still works if fromEdge is split which isn't forward edge but backward edge + //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops + @Test + public void testTurnRestrictionWithSplitOnFromBackwardEdge() throws Exception { + + TurnRestriction restriction = new TurnRestriction(); + restriction.fromEdge = 45; + restriction.toEdge = 28; + restriction.only = false; + int ridx = sl.turnRestrictions.size(); + sl.turnRestrictions.add(restriction); + sl.edgeStore.turnRestrictions.put(restriction.fromEdge, ridx); + sl.addReverseTurnRestriction(restriction, ridx); + + + ProfileRequest profileRequest = new ProfileRequest(); + profileRequest.fromLat = 38.89098; + profileRequest.fromLon = -76.99478; + profileRequest.toLat = 38.891657; + profileRequest.toLon = -76.99661326; + + StreetRouter streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + StreetRouter.State lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + + int oldWeight = lastState.weight; + StreetRouter.State state = lastState; + EdgeStore.Edge edge = sl.edgeStore.getCursor(); + + TurnRestriction tr = sl.turnRestrictions.get(2); + + LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getLengthMm()); + //Turn from 45 to 28 is in Turn restriction 153 is new splitted edge + assertFalse(state.backEdge == 28 && (state.backState.backEdge == 45 || state.backState.backEdge == 153)); + state = state.backState; + + } + + //Splits "from Turn restriction edge" + int vertex = sl.createAndLinkVertex(38.8909806,-76.995403); + + sl.buildEdgeLists(); + + streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + state = lastState; + + int newWeight = lastState.weight; + + //Weight isn't exactly the same because edge is split and some decimals are forgotten + // when weight is summed since it's an integer not float. + assertEquals(oldWeight, newWeight+1); + + + //LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + //LOG.info("V:{} W:{} be:{}, flags:{}", state.vertex, state.weight, state.backEdge, edge.getFlagsAsString()); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getLengthMm()); + //Turn from 45 to 28 is in Turn restriction 153 is new splitted edge + assertFalse(state.backEdge == 28 && (state.backState.backEdge == 45 || state.backState.backEdge == 153)); + state = state.backState; + + } + + } + //Test if turn restriction still works if toEdge is split //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops @Test From 23b0dd744a5409047a5b722f3589ef75a756cb8e Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Thu, 13 Apr 2017 11:40:29 +0200 Subject: [PATCH 10/14] Fixes bug when we split to turn restriction backward edge Also adds tests --- .../com/conveyal/r5/streets/StreetLayer.java | 26 +++++ .../conveyal/r5/streets/TurnSplitTest.java | 94 +++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 25ff9812e..2b4734485 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -1205,6 +1205,32 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) return true; }); + // clean up any turn restrictions that exist on this edge as to edge + // turn restrictions on the backward edge go to the new edge's backward edge. Turn restrictions on the forward edge stay + // where they are + edgeStore.turnRestrictionsReverse.removeAll(split.edge+1).forEach(ridx -> { + TurnRestriction tr = turnRestrictions.get(ridx); + TurnRestriction newTurnRestriction; + // If edge is mutable we are splitting normal edge and can modify existing turnRestriction with new toEdge + if (edge.isMutable()) { + newTurnRestriction = tr; + } else { + //If it isn't we are in scenario. Which means we need to replace current turnRestriction with new one + //Since TurnRestriction in original graph needs to stay the same + newTurnRestriction = new TurnRestriction(tr); + } + //+1 since edgeIndex is forward edge but we need backward edge + newTurnRestriction.toEdge = newEdge1.edgeIndex+1; + + //In scenario we replace existing turnRestriction with new one + if (!edge.isMutable()) { + turnRestrictions.remove(ridx); + turnRestrictions.add(ridx, newTurnRestriction); + } + edgeStore.turnRestrictionsReverse.put(newEdge1.edgeIndex+1, ridx); + return true; + }); + //Turn restrictions that are on via edge should be updated. Since we now have 2 via edges from one, because we split one edgeStore.turnRestrictionsVia.get(split.edge).forEach(turnRestrictionIndex -> { TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index c08ca6bee..3706590b1 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -293,6 +293,100 @@ public void testTurnRestrictionWithSplitOnTo() throws Exception { } + //Test if turn restriction still works if toEdge is split which isn't forward edge but backward edge + //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops + @Test + public void testTurnRestrictionWithSplitOnToBackwardEdge() throws Exception { + + TurnRestriction restriction = new TurnRestriction(); + restriction.fromEdge = 47; + restriction.toEdge = 45; + restriction.only = false; + int ridx = sl.turnRestrictions.size(); + sl.turnRestrictions.add(restriction); + sl.edgeStore.turnRestrictions.put(restriction.fromEdge, ridx); + sl.addReverseTurnRestriction(restriction, ridx); + + + ProfileRequest profileRequest = new ProfileRequest(); + profileRequest.fromLat = 38.89098; + profileRequest.fromLon = -76.99478; + profileRequest.toLat = 38.891657; + profileRequest.toLon = -76.99661326; + + StreetRouter streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + StreetRouter.State lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + + int oldWeight = lastState.weight; + StreetRouter.State state = lastState; + EdgeStore.Edge edge = sl.edgeStore.getCursor(); + + TurnRestriction tr = sl.turnRestrictions.get(2); + + LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getLengthMm()); + //Turn from 47 to 45 is in Turn restriction 153 is new splitted edge + assertFalse(state.backEdge == 45 && (state.backState.backEdge == 47 || state.backState.backEdge == 153)); + state = state.backState; + + } + + //Splits "from Turn restriction edge" + int vertex = sl.createAndLinkVertex(38.8909806,-76.995403); + + sl.buildEdgeLists(); + + streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + state = lastState; + + int newWeight = lastState.weight; + + //Weight isn't exactly the same because edge is split and some decimals are forgotten + // when weight is summed since it's an integer not float. + assertEquals(oldWeight, newWeight); + + + LOG.info("TR:{}->{}", tr.fromEdge, tr.toEdge); + while (state != null) { + edge.seek(state.backEdge); + //LOG.info("V:{} W:{} be:{}, flags:{}", state.vertex, state.weight, state.backEdge, edge.getFlagsAsString()); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getLengthMm()); + //Turn from 47 to 45 is in Turn restriction 153 is new splitted edge + assertFalse(state.backEdge == 45 && (state.backState.backEdge == 47 || state.backState.backEdge == 153)); + state = state.backState; + + } + + } + //Test if turn restriction still works if viaEdge is split //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops From 85912e0a48ccdd22c1cb9f438d199db1468e3ac8 Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Thu, 13 Apr 2017 12:19:59 +0200 Subject: [PATCH 11/14] Fixed wrong OSM data bug in tests It could happen that two different tests that were using FakeGraph with different OSM data were actually using the same data, since both create osm.mapdb in /tmp. And if file already exists it is assumed it's from previous run, but it's actually different graph. This adds temporary parameter to TransportNetwork#fromFiles which creates temporary osm.mapdb which is removed after creation if temporary is true. Default is false since removal should only happen in tests. --- .../conveyal/r5/transit/TransportNetwork.java | 20 +++++++++++-------- .../r5/analyst/scenario/FakeGraph.java | 6 ++++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java index f156101cc..9d2ff0d25 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java @@ -130,12 +130,12 @@ public void rebuildTransientIndexes() { /** Create a TransportNetwork from gtfs-lib feeds */ public static TransportNetwork fromFeeds (String osmSourceFile, List feeds, TNBuilderConfig config) { - return fromFiles(osmSourceFile, null, feeds, config); + return fromFiles(osmSourceFile, null, feeds, config, false); } /** Legacy method to load from a single GTFS file */ public static TransportNetwork fromFiles (String osmSourceFile, String gtfsSourceFile, TNBuilderConfig tnBuilderConfig) throws DuplicateFeedException { - return fromFiles(osmSourceFile, Arrays.asList(gtfsSourceFile), tnBuilderConfig); + return fromFiles(osmSourceFile, Arrays.asList(gtfsSourceFile), tnBuilderConfig, false); } /** @@ -144,8 +144,8 @@ public static TransportNetwork fromFiles (String osmSourceFile, String gtfsSourc * the feeds into memory simulataneously, which shouldn't be so bad with mapdb-based feeds, but it's still not great * (due to caching etc.) */ - private static TransportNetwork fromFiles (String osmSourceFile, List gtfsSourceFiles, List feeds, - TNBuilderConfig tnBuilderConfig) throws DuplicateFeedException { + private static TransportNetwork fromFiles(String osmSourceFile, List gtfsSourceFiles, + List feeds, TNBuilderConfig tnBuilderConfig, boolean temporary) throws DuplicateFeedException { System.out.println("Summarizing builder config: " + BUILDER_CONFIG_FILENAME); System.out.println(tnBuilderConfig); @@ -154,8 +154,9 @@ private static TransportNetwork fromFiles (String osmSourceFile, List gt // Create a transport network to hold the street and transit layers TransportNetwork transportNetwork = new TransportNetwork(); + String osmPath = temporary ? null : new File(dir,"osm.mapdb").getPath(); // Load OSM data into MapDB - OSM osm = new OSM(new File(dir,"osm.mapdb").getPath()); + OSM osm = new OSM(osmPath); osm.intersectionDetection = true; osm.readFromFile(osmSourceFile); @@ -218,9 +219,12 @@ private static TransportNetwork fromFiles (String osmSourceFile, List gt * On the other hand, GTFS feeds each have their own namespace. Each GTFS object is for one specific feed, and this * distinction should be maintained for various reasons. However, we use the GTFS IDs only for reference, so it * doesn't really matter, particularly for analytics. + * + * @param temporary: if true osm.mapdb is deleted after use. Default is false since it is needed for way names */ - public static TransportNetwork fromFiles (String osmFile, List gtfsFiles, TNBuilderConfig config) { - return fromFiles(osmFile, gtfsFiles, null, config); + public static TransportNetwork fromFiles(String osmFile, List gtfsFiles, + TNBuilderConfig config, boolean temporary) { + return fromFiles(osmFile, gtfsFiles, null, config, temporary); } public static TransportNetwork fromDirectory (File directory) throws DuplicateFeedException { @@ -254,7 +258,7 @@ public static TransportNetwork fromDirectory (File directory) throws DuplicateFe LOG.error("An OSM PBF file is required to build a network."); return null; } else { - return fromFiles(osmFile.getAbsolutePath(), gtfsFiles, builderConfig); + return fromFiles(osmFile.getAbsolutePath(), gtfsFiles, builderConfig, false); } } diff --git a/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java b/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java index 63677f9ec..e1768c23b 100644 --- a/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java +++ b/src/test/java/com/conveyal/r5/analyst/scenario/FakeGraph.java @@ -64,7 +64,8 @@ public static TransportNetwork buildNetwork (TransitNetwork... networks) { gtfsFiles.add(gtfsFile.getAbsolutePath()); } - TransportNetwork net = TransportNetwork.fromFiles(osmFile.getAbsolutePath(), gtfsFiles, new TNBuilderConfig()); + TransportNetwork net = TransportNetwork.fromFiles(osmFile.getAbsolutePath(), gtfsFiles, new TNBuilderConfig(), + true); net.transitLayer.buildDistanceTables(null); // clean up @@ -100,7 +101,8 @@ public static TransportNetwork buildNetwork (InputStream osmFileStream, String o gtfsFiles.add(gtfsFile.getAbsolutePath()); } - TransportNetwork net = TransportNetwork.fromFiles(osmFile.getAbsolutePath(), gtfsFiles, tnBuilderConfig); + TransportNetwork net = TransportNetwork.fromFiles(osmFile.getAbsolutePath(), gtfsFiles, tnBuilderConfig, + true); net.transitLayer.buildDistanceTables(null); // clean up From 4abb0f3c62c80e6fe1e3f258610e79e750b7368b Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Thu, 13 Apr 2017 13:15:24 +0200 Subject: [PATCH 12/14] Fixes bug when we split via turn restriction backward edge Also tests added. --- .../com/conveyal/r5/streets/StreetLayer.java | 23 +++++ .../conveyal/r5/streets/TurnSplitTest.java | 96 +++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 2b4734485..c91c76125 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -1254,6 +1254,29 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) return true; }); + //Turn restrictions that are on via backward edge should be updated. Since we now have 2 via edges from one, because we split one + edgeStore.turnRestrictionsVia.get(split.edge+1).forEach(turnRestrictionIndex -> { + TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); + TurnRestriction newTurnRestriction; + int currentViaEdgeIndex = ArrayUtils.indexOf(tr.viaEdges, split.edge+1); + //New via edge index is added as the prev edge in viaEdges list + //noinspection UnnecessaryLocalVariable + int[] newViaEdges = ArrayUtils.add(tr.viaEdges, currentViaEdgeIndex, newEdge1.edgeIndex+1); + if (edge.isMutable()) { + newTurnRestriction = tr; + } else { + newTurnRestriction = new TurnRestriction(tr); + } + newTurnRestriction.viaEdges = newViaEdges; + if (!edge.isMutable()) { + turnRestrictions.remove(turnRestrictionIndex); + turnRestrictions.add(turnRestrictionIndex, newTurnRestriction); + } + //Also updates turnRestrictionsViaList + edgeStore.turnRestrictionsVia.put(newEdge1.edgeIndex+1, turnRestrictionIndex); + return true; + }); + // Insert the new edge into the spatial index if (!edgeStore.isExtendOnlyCopy()) { diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index 3706590b1..c06d5fa53 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -473,4 +473,100 @@ public void testTurnRestrictionWithSplitOnVia() throws Exception { assertEquals(oldWeight, newWeight); } + + //Test if turn restriction still works if viaEdge is split which isn't forward edge but backward edge + //This splitting happens in StreetLayer#getOrCreateVertexNear which is used in associateBikeSharing, buildParkAndRideNodes and associateStops + @Test + public void testTurnRestrictionWithSplitOnViaBackwardEdge() throws Exception { + + TurnRestriction restriction = new TurnRestriction(); + restriction.fromEdge = 47; + restriction.toEdge = 28; + restriction.viaEdges = new int[]{45}; + restriction.only = false; + int ridx = sl.turnRestrictions.size(); + sl.turnRestrictions.add(restriction); + sl.edgeStore.turnRestrictions.put(restriction.fromEdge, ridx); + sl.edgeStore.turnRestrictionsVia.put(restriction.viaEdges[0], ridx); + sl.addReverseTurnRestriction(restriction, ridx); + + + ProfileRequest profileRequest = new ProfileRequest(); + profileRequest.fromLat = 38.89098; + profileRequest.fromLon = -76.99478; + profileRequest.toLat = 38.891657; + profileRequest.toLon = -76.99661326; + + StreetRouter streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + StreetRouter.State lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + + int oldWeight = lastState.weight; + StreetRouter.State state = lastState; + EdgeStore.Edge edge = sl.edgeStore.getCursor(); + + TurnRestriction tr = sl.turnRestrictions.get(2); + + LOG.info("TR:{}", tr); + while (state != null) { + edge.seek(state.backEdge); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getLengthMm()); + //Turn from 47 to 45 is in Turn restriction 153 is new splitted edge + assertFalse(state.backEdge == 28 && (state.backState.backEdge == 45)); + state = state.backState; + + } + + //Splits "from Turn restriction edge" + int vertex = sl.createAndLinkVertex(38.8909806,-76.995403); + + sl.buildEdgeLists(); + + streetRouter = new StreetRouter(sl); + streetRouter.profileRequest = profileRequest; + streetRouter.streetMode = StreetMode.CAR; + + + + streetRouter.distanceLimitMeters = 5_000; + //Split for end coordinate + assertTrue("Destination must be found", streetRouter.setDestination(profileRequest.toLat, profileRequest.toLon)); + assertTrue("Origin must be found", streetRouter.setOrigin(profileRequest.fromLat, profileRequest.fromLon)); + + streetRouter.route(); + + lastState = streetRouter.getState(streetRouter.getDestinationSplit()); + //LOG.info("W:{}", lastState.weight); + state = lastState; + + int newWeight = lastState.weight; + + //Weight isn't exactly the same because edge is split and some decimals are forgotten + // when weight is summed since it's an integer not float. + assertEquals(oldWeight, newWeight+1); + + + LOG.info("TR:{}", tr); + while (state != null) { + edge.seek(state.backEdge); + //LOG.info("V:{} W:{} be:{}, flags:{}", state.vertex, state.weight, state.backEdge, edge.getFlagsAsString()); + LOG.debug("V:{} W:{} be:{}, TR:{} flags:{}", state.vertex, state.weight, state.backEdge, sl.edgeStore.turnRestrictions.containsKey(state.backEdge), edge.getLengthMm()); + //Turn from 47 to 45 is in Turn restriction 153 is new splitted edge + assertFalse(state.backEdge == 28 && (state.backState.backEdge == 45)); + state = state.backState; + + } + + } } From 08d0eeb594f4f9aabe4dbef29b09d418616a9763 Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Thu, 20 Apr 2017 14:26:28 +0200 Subject: [PATCH 13/14] Fixes bug when we split from/to edges on ONLY turn restrictions Also tests are added --- .../PointToPointRouterServer.java | 26 +++++- .../com/conveyal/r5/streets/EdgeStore.java | 34 +++++++- .../com/conveyal/r5/streets/StreetLayer.java | 53 +++++++++++++ .../conveyal/r5/streets/TurnSplitTest.java | 79 +++++++++++++++++++ 4 files changed, 188 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java b/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java index 34a39320c..8f641ce65 100644 --- a/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java +++ b/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java @@ -96,6 +96,22 @@ public static void main(String[] commandArguments) { LOG.info("Loading transit networks from: {}", dir); TransportNetwork transportNetwork = TransportNetwork.read(new File(dir, "network.dat")); transportNetwork.readOSM(new File(dir, "osm.mapdb")); + int invalidTrs = 0; + for (int i=0; i < transportNetwork.streetLayer.turnRestrictions.size(); i++) { + TurnRestriction tr = transportNetwork.streetLayer.turnRestrictions.get(i); + if (tr.viaEdges.length > 0) { + continue; + } + EdgeStore.Edge fromEdge = transportNetwork.streetLayer.edgeStore.getCursor(tr.fromEdge); + EdgeStore.Edge toEdge = transportNetwork.streetLayer.edgeStore.getCursor(tr.toEdge); + + int viaVertex = fromEdge.getToVertex(); + if(!transportNetwork.streetLayer.outgoingEdges.get(viaVertex).contains(tr.toEdge)) { + invalidTrs++; + LOG.info("TR:{} -> {}, {}", fromEdge.getOSMID(), toEdge.getOSMID(), tr); + } + } + LOG.info("Invalid trs:{}", invalidTrs); run(transportNetwork); } catch (Exception e) { LOG.error("An error occurred during the reading or decoding of transit networks", e); @@ -723,7 +739,6 @@ private static void run(TransportNetwork transportNetwork) { edgeIdx++; makeTurnEdge(transportNetwork, both, features, cursor, offsetBuilder, distance, edgeIdx); - return true; } catch (Exception e) { response.status(500); @@ -1068,6 +1083,15 @@ private static void makeTurnEdge(TransportNetwork transportNetwork, boolean both for (int i=0; i < edge_restricion_idxs.size(); i++) { int turnRestrictionIdx = edge_restricion_idxs.get(i); TurnRestriction turnRestriction = transportNetwork.streetLayer.turnRestrictions.get(turnRestrictionIdx); + if (turnRestriction.viaEdges.length > 0) { + continue; + } + EdgeStore.Edge fromEdge = transportNetwork.streetLayer.edgeStore.getCursor(turnRestriction.fromEdge); + + int viaVertex = fromEdge.getToVertex(); + if(transportNetwork.streetLayer.outgoingEdges.get(viaVertex).contains(turnRestriction.toEdge)) { + continue; + } //TurnRestriction.fromEdge isn't necessary correct //If edge on which from is is splitted then fromEdge is different but isn't updated in TurnRestriction diff --git a/src/main/java/com/conveyal/r5/streets/EdgeStore.java b/src/main/java/com/conveyal/r5/streets/EdgeStore.java index e37d37b52..8beaf19a2 100644 --- a/src/main/java/com/conveyal/r5/streets/EdgeStore.java +++ b/src/main/java/com/conveyal/r5/streets/EdgeStore.java @@ -139,13 +139,35 @@ public boolean isExtendOnlyCopy() { } - /** Turn restrictions for turning _out of_ each edge */ + /** Turn restrictions for turning _out of_ each edge + * + * FROM edges of all turn restrictions + * */ public TIntIntMultimap turnRestrictions; - /** Turn restrictions for turning _in of_ each edge */ + /** Turn restrictions for turning _in of_ each edge in reverse search + * + * TO edges of NO TURN restrictions + * and TO edges of NO TURN restrictions remapped from ONLY TURN restrictions + * + * Also used when splitting edges + * */ public TIntIntMultimap turnRestrictionsReverse; - /** Via edges to turn restrictions link. Used when splitting edges + /** + * TO edges of ONLY TURN restrictions. (They aren't in turnRestrictionsReverse since reverse search + * doesn't support ONLY TURNS) + * Used only when splitting edges + * */ + public TIntIntMultimap turnRestrictionsToEdges; + + /** + * Here are FROM edges of NO TURN restriction which are remapped ONLY turn restrictions to NO turn restrictions + * Used only when splitting edges + */ + public TIntIntMultimap turnRestrictionsFromEdges; + + /** Via edges to turn restrictions link. Used only when splitting edges * FIXME: Can be probably transient since it's used only when building graph in getOrCreateVertexNear * But I'm not sure what is with scenarios (materializeOne function)*/ public TIntIntMultimap turnRestrictionsVia; @@ -175,6 +197,8 @@ public EdgeStore (VertexStore vertexStore, StreetLayer layer, int initialSize) { outAngles = new TByteArrayList(initialEdgePairs); turnRestrictions = new TIntIntHashMultimap(); turnRestrictionsReverse = new TIntIntHashMultimap(); + turnRestrictionsToEdges = new TIntIntHashMultimap(); + turnRestrictionsFromEdges = new TIntIntHashMultimap(); turnRestrictionsVia = new TIntIntHashMultimap(); } @@ -1073,6 +1097,10 @@ public EdgeStore extendOnlyCopy() { (TIntIntHashMultimap) turnRestrictionsReverse); copy.turnRestrictionsVia = new TIntIntHashMultimap( (TIntIntHashMultimap) turnRestrictionsVia); + copy.turnRestrictionsToEdges = new TIntIntHashMultimap( + (TIntIntHashMultimap) turnRestrictionsToEdges); + copy.turnRestrictionsFromEdges = new TIntIntHashMultimap( + (TIntIntHashMultimap) turnRestrictionsFromEdges); return copy; } diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index c91c76125..22bac4e51 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -901,6 +901,8 @@ else if ("via".equals(member.role)) { */ void addReverseTurnRestriction(TurnRestriction turnRestriction, int index) { if (turnRestriction.only) { + //Adds To edges of ONLY turn restriction to map of too edges. This is used in splitting + edgeStore.turnRestrictionsToEdges.put(turnRestriction.toEdge, index); //From Only turn restrictions create multiple NO TURN restrictions which means the same //Since only turn restrictions aren't supported in reverse street search List remapped = turnRestriction.remap(this); @@ -908,6 +910,7 @@ void addReverseTurnRestriction(TurnRestriction turnRestriction, int index) { index = turnRestrictions.size(); turnRestrictions.add(remapped_restriction); edgeStore.turnRestrictionsReverse.put(remapped_restriction.toEdge, index); + edgeStore.turnRestrictionsFromEdges.put(remapped_restriction.fromEdge, index); } } else { edgeStore.turnRestrictionsReverse.put(turnRestriction.toEdge, index); @@ -1205,6 +1208,29 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) return true; }); + + edgeStore.turnRestrictionsFromEdges.removeAll(split.edge).forEach(ridx -> { + TurnRestriction tr = turnRestrictions.get(ridx); + TurnRestriction newTurnRestriction; + // If edge is mutable we are splitting normal edge and can modify existing turnRestriction with new fromEdge + if (edge.isMutable()) { + newTurnRestriction = tr; + } else { + //If it isn't we are in scenario. Which means we need to replace current turnRestriction with new one + //Since TurnRestriction in original graph needs to stay the same + newTurnRestriction = new TurnRestriction(tr); + } + newTurnRestriction.fromEdge = newEdge1.edgeIndex; + + //In scenario we replace existing turnRestriction with new one + if (!edge.isMutable()) { + turnRestrictions.remove(ridx); + turnRestrictions.add(ridx, newTurnRestriction); + } + edgeStore.turnRestrictionsFromEdges.put(newEdge1.edgeIndex, ridx); + return true; + }); + // clean up any turn restrictions that exist on this edge as to edge // turn restrictions on the backward edge go to the new edge's backward edge. Turn restrictions on the forward edge stay // where they are @@ -1231,6 +1257,33 @@ public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) return true; }); + + // clean up any turn restrictions that exist on this edge as to edge + // turn restrictions on the backward edge go to the new edge's backward edge. Turn restrictions on the forward edge stay + // where they are + edgeStore.turnRestrictionsToEdges.removeAll(split.edge+1).forEach(ridx -> { + TurnRestriction tr = turnRestrictions.get(ridx); + TurnRestriction newTurnRestriction; + // If edge is mutable we are splitting normal edge and can modify existing turnRestriction with new toEdge + if (edge.isMutable()) { + newTurnRestriction = tr; + } else { + //If it isn't we are in scenario. Which means we need to replace current turnRestriction with new one + //Since TurnRestriction in original graph needs to stay the same + newTurnRestriction = new TurnRestriction(tr); + } + //+1 since edgeIndex is forward edge but we need backward edge + newTurnRestriction.toEdge = newEdge1.edgeIndex+1; + + //In scenario we replace existing turnRestriction with new one + if (!edge.isMutable()) { + turnRestrictions.remove(ridx); + turnRestrictions.add(ridx, newTurnRestriction); + } + edgeStore.turnRestrictionsToEdges.put(newEdge1.edgeIndex+1, ridx); + return true; + }); + //Turn restrictions that are on via edge should be updated. Since we now have 2 via edges from one, because we split one edgeStore.turnRestrictionsVia.get(split.edge).forEach(turnRestrictionIndex -> { TurnRestriction tr = turnRestrictions.get(turnRestrictionIndex); diff --git a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java index c06d5fa53..10321c9d3 100644 --- a/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java +++ b/src/test/java/com/conveyal/r5/streets/TurnSplitTest.java @@ -569,4 +569,83 @@ public void testTurnRestrictionWithSplitOnViaBackwardEdge() throws Exception { } } + + // Test if turn restriction is valid if from edge on ONLY turn is split + // This is different from normal from edge since ONLY turns from edges aren't saved in TurnRestriction list + @Test + public void testTurnRestrictionWithSplitOnFromOnly() throws Exception { + TurnRestriction out = new TurnRestriction(); + out.fromEdge = 134; + out.toEdge = 31; + out.only = true; + int index = sl.turnRestrictions.size(); + sl.turnRestrictions.add(out); + sl.edgeStore.turnRestrictions.put(out.fromEdge, index); + sl.addReverseTurnRestriction(out, index); + for (int i =index; i < sl.turnRestrictions.size(); i++) { + TurnRestriction tr = sl.turnRestrictions.get(i); + LOG.info("TR:{} {}", i, tr); + testTurnRestriction(tr); + } + + //Splits FROM edge + sl.createAndLinkVertex(38.8919723,-76.9959763); + sl.buildEdgeLists(); + LOG.info("Splits from edge on TR"); + + for (int i =index; i < sl.turnRestrictions.size(); i++) { + TurnRestriction tr = sl.turnRestrictions.get(i); + if (i == index) { + assertFalse("New Turn restriction should have different FROM edge", tr.fromEdge == 134); + } + testTurnRestriction(tr); + } + + } + + // Test if turn restriction is valid if to edge on ONLY turn is split + // This is different from normal to edge test since ONLY turns to edges aren't saved in TurnRestrictionReverse list + @Test + public void testTurnRestrictionWithSplitOnToOnly() throws Exception { + TurnRestriction out = new TurnRestriction(); + out.fromEdge = 134; + out.toEdge = 31; + out.only = true; + int index = sl.turnRestrictions.size(); + sl.turnRestrictions.add(out); + sl.edgeStore.turnRestrictions.put(out.fromEdge, index); + sl.addReverseTurnRestriction(out, index); + for (int i =index; i < sl.turnRestrictions.size(); i++) { + TurnRestriction tr = sl.turnRestrictions.get(i); + testTurnRestriction(tr); + } + + //Splits TO edge + sl.createAndLinkVertex(38.891768,-76.9962057); + sl.buildEdgeLists(); + LOG.info("Splits to edge on TR"); + + for (int i =index; i < sl.turnRestrictions.size(); i++) { + TurnRestriction tr = sl.turnRestrictions.get(i); + if (i == index) { + assertFalse("New Turn restriction should have different TO edge", tr.toEdge == 31); + } + testTurnRestriction(tr); + } + + + + } + + public void testTurnRestriction(TurnRestriction tr) { + EdgeStore.Edge fromEdge = sl.edgeStore.getCursor(tr.fromEdge); + EdgeStore.Edge toEdge = sl.edgeStore.getCursor(tr.toEdge); + + int viaVertex = fromEdge.getToVertex(); + LOG.info("TR:{} -> {}, {}", fromEdge.getOSMID(), toEdge.getOSMID(), tr); + //LOG.info ("Frome edge distance:{}", fromEdge.getLengthMm()); + assertTrue(sl.outgoingEdges.get(viaVertex).contains(tr.toEdge)); + + + } } From e624c5e1383bc3b78f04b9897d705381a41b1ea1 Mon Sep 17 00:00:00 2001 From: Marko Burjek Date: Fri, 21 Apr 2017 16:03:30 +0200 Subject: [PATCH 14/14] Via edges from remapped Turn Restrictions are saved --- src/main/java/com/conveyal/r5/streets/StreetLayer.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index 22bac4e51..c7c2f61e5 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -909,6 +909,9 @@ void addReverseTurnRestriction(TurnRestriction turnRestriction, int index) { for (TurnRestriction remapped_restriction: remapped) { index = turnRestrictions.size(); turnRestrictions.add(remapped_restriction); + for(int edgeId: turnRestriction.viaEdges) { + edgeStore.turnRestrictionsVia.put(edgeId, index); + } edgeStore.turnRestrictionsReverse.put(remapped_restriction.toEdge, index); edgeStore.turnRestrictionsFromEdges.put(remapped_restriction.fromEdge, index); }