Hello, sorry for late reply. TraCI communication based on protobuffers would not resolve the question of messy interface: what I have meant concerns application level. It is not that individual get/set methods would be badly designed. It is that it is messy to use on the whole: there are corner cases to avoid, interdependencies and sometimes unintuitive behavior. It seems that it was not designed as a whole, but functionalities were added on top of each other over time. Then naturally their integration is loose.
What I would like is to have a *minimal* interface where individual functionalities of SUMO do not overlap more than necesarry (no interdependencies). Note that I am talking about application level, not socket level. This would have benefits for both devs and users. The users will have an easy time to use it and will not need to go through complicated documentation (surely you noticed that your users often don't read properly your wiki, or don't find what they were looking for even it is there, or just get confused). The developers on the other hand will not have as many bugs on their hands due to reduced complexity of such interface. But this also requires (1) limited the backward compatibility; (2) removal of all functions that can be implemented on user-level; (3) limiting the options the user has. As an example how to avoid the bloatness you talked about consider the SQLite project. Each version of SQLite has own API designed from scratch. But within a single version the interface is kept backward compatible so the users can update to the new version without adapting their codebase. This kind of hybrid approach might be the right solution for you: if you find a problem and face the dilemma whether to keep backward compatibility then you need to keep it only within current major version of SUMO and design things better-integrated for the next major version. You asked about some specific issues, but I prefer not to give examples as that would help treat symptoms rather than causes. But what I am talking about does not limit to TraCI. The net file is a pain to deal with for example. I know that you recommend to use netconvert instead of meddling with net file XML, but notice that if you would have suficiently simple net representation you would actually not need such a generic tool as netconvert seems to be - people would simply adapt their net files manually or through own scripts. Finally, let me finish with an example of my experience with SUMO: today I wanted to adapt my tools to work with new LuST2.0 ... and I have stumbled upon a bug. I am sure you will resolve it quickly, but this is the kind of experience I am getting consistently. I have already said that I owe SUMO a great deal and don't misunderstand me: even working with it was painful at times, I do not regret using it. It needed a lot of persuasion, but it delivers in full. Matej. On 17.11.2016 12:19, Jakob Erdmann wrote: > Hello Matêj, > thanks for the input. We have ideas for implementing a TraCI 2.0 based > on google protocol buffers which might be the fresh cut you envision. > For now we have a gradual evolution of the APIs and each addition > brings the dilemma of choosing between bloat and loss of backward > compatibility. > > It would help a great deal if you could offer concrete examples of > ambiguity and messiness or even better, suggestions for improvement. > At last cleaning up the documentation to reduce ambiguity would be a > start. > > Could any of the code you wrote to work around our library problems be > folded back into the library? > > For the concrete case of 3D support in sumolib, assuming that you used > it's functionality at some point, > would you rather rewrite all your scripts because node.getPosition() > suddenly returns 3-tuples or would you prefer interface bloat due to a > new method getPosition3D()? > > best regards, > Jakob > > > 2016-11-17 10:53 GMT+01:00 Matěj Kubička > <matej.kubi...@l2s.centralesupelec.fr > <mailto:matej.kubi...@l2s.centralesupelec.fr>>: > > Hello Jakob et al., > if I may put my two cents in: I wanted to use SUMO's 3D support > about a > year ago, but the whole thing looked messy to me. I never arrived to a > point where I would feel comfortable using it. Instead I went on to > implement what I needed manually. > > While SUMO is a good simulator its interfaces often feel messy and > ambiguous. If I would appreciate something as a user it would be > if you > could drop the backward compatibility at some point and provide fresh > clean-cut interfaces. > > On one hand SUMO provided exactly what I needed for my work, but > getting > it to work the way I wanted was painful. I have spent over three > months > just developing and debugging my experiments: tracking various bugs, > dealing with spurious crashes, adapting TraCI client.. It was > worth it, > but it could have potentially been done much faster. If I can polemize > on why it was so difficult I would say it was because the > interfaces are > heavy. They allow too many variations of parameters, often in > ambiguous > way. This makes them difficult to maintain for you (hence the > bugs), and > difficult to use for me. > > I owe great deal to SUMO, so don't take this the wrong way: I hope to > give you feedback, it is not to criticize pointlessly. > > Matej. > > > > On 17.11.2016 09:16, Jakob Erdmann wrote: > > Hello, > > there are 2 points where I disagree with you: > > - breaking the code that works with 2D networks when run on a 3D > network is > > not necessarily a good thing. There is any number of reasonable > things to > > be done with 2D geometry in a 3D network. > > - controlling return types by parameter is less readable than having > > dedicated function names. consider reading the following > statements in > > source: > > pos = node.getPosition(true) > > vs > > pos = node.getPosition3D() > > > > Note only is the 'true' surprising (why would a position > function have an > > extra parameter?), it also doesn't reveal what it is doing. > > By your suggestion, all "clean" scripts would use > getPosition(true) from > > now on. > > > > I will stand by the implementation plan 3) from my previous mail > of having > > getPosition() behave as before and getPosition3D() always > returning a > > 3-tuple. > > > > regards, > > Jakob > > > > 2016-11-16 11:01 GMT+01:00 sumo.mahei <sumo.ma...@googlemail.com > <mailto:sumo.ma...@googlemail.com>>: > > > >> Hi Jacob, > >> > >> I was thinking about something like: > >> > >> def getPosition('node_id', withZ=False): > >> if len(self._coord) == 2 and withZ: # fake the non > existing > >> z-coord > >> return (self._coords[0], self.coord[1], 0) > >> else: # > >> return 3d if node is 3d (also for withZ) > >> return self._coord # return 2d fi > >> node is 2d and not withZ > >> > >> If you want your new code/scripts to be clean, then use > withZ=True from > >> now on. > >> This always returns x,y,z - save on any net and less code in > your script. > >> > >> It doesn't break your old code on pure 2d nets, > >> because your scripts have been eating 2d before and > >> are feet now again with 2d only since the net is 2d only. > >> > >> It will break your old scripts on 3d-networks, > >> which is __good__ because quite likely you've done > >> something wrong in the past. > >> > >> If that was on purpose, > >> well then use the 2D-Wrapper with search and replace as you > suggested. > >> > >> > >> Am 16.11.2016 um 08:45 schrieb Jakob Erdmann: > >>> Hello, > >>> > >>> 1) the advantage of having 3D and 2D variants of geometry are > threefold: > >>> - it follows the TraCI API which provides 2 distinct methods for > >>> returning 2D and 3D geometry > >>> (http://sumo.dlr.de/wiki/TraCI/Vehicle_Value_Retrieval > <http://sumo.dlr.de/wiki/TraCI/Vehicle_Value_Retrieval>). This is only > >>> implemented for traci.person.getPosition / getPosition3D in > the python > >>> client at the moment in case you are wondering why you never > saw it. > >> -> I experienced this as a drawback of TraCI, I do more look at > SUMO and > >> netconvert than on TraCI as reference > >> > >> - it allows trivial search/replace conversion for existing code > >> -> agree, see below > >> > >>> - it follows the coding style in src/utils/geom with distinct > methods > >>> for handling 2D and 3D geometry > >> -> not my playground so far :-) > >> -> we may additionally include wrappers with endings 2D and 3D > if that > >> closes the gap > >> > >>> 2) the disadvantage of determining function return types at > runtime, > >>> is that it makes code that handles arbitrary networks much > harder to > >>> write (you suddenly need to check types when calling geometry > >>> functions as your example shows) > >> -> agree 100% and agian 100% that is why suggested the use > withZ=True as > >> described yesterday and again above for arbitrary net-scripting > >> b.t.w. checking always the dim is truly very annoying and that > is the > >> reason why I did write some sumolibExtensions some time ago. > >> > >>> 3) you raise an important point regarding backwards compatibility. > >>> I'm not sure how many people use tuple unpacking in their code > but I > >>> agree that it's bad to break working code. > >> -> ??? you can never tell :-) I know you did use unpacking in your > >> scripts :-) > >> > >>> An alternative would be to have the old client methods continue to > >>> return 2D geometry and only add a new method with the 3D suffix > >>> (exactly as in traci.person but somewhat different than in > >>> src/utils/geom). > >>> > >> -> this might let your (future) users to stumble across the missing > >> z-coord where they expect them. They might think this is a bug > unless > >> they dive into the docs > >> to find out. Not the nicest coding experience. > >> > >>> best regards, > >>> Jakob > >>> > >>> > >> So which way to go? > >> I suggest: > >> > >> * return geometry tuples according to the largest dim of the net > >> (either always 2d on a pure 2d net or always 3d on a net > with at > >> least one node's z!=0) > >> -> requires a tiny post process after loading the xml data. > >> > >> * geometry return can switch on the z-layer optionally for 2d > nets for > >> ease of coding > >> > >> * have getSomething2D() and getSomething3D() - Wrappers for all > geometry > >> returning functions, > >> brute forcing 2d and 3d tuples even if information is lost. > >> > >> Would that be fine for you? > >> > >> > >> Best Marek Heinrich > >> simoserv GmbH > >> > >> > >> > >>> > >>> > >>> > >>> 2016-11-15 19:11 GMT+01:00 sumo.mahei > <sumo.ma...@googlemail.com <mailto:sumo.ma...@googlemail.com> > >>> <mailto:sumo.ma...@googlemail.com > <mailto:sumo.ma...@googlemail.com>>>: > >>> > >>> Dear Jacob, > >>> > >>> maybe it's worth putting the discussion about Ticket 2072 > >>> from private mailing to the sumo-devel list. > >>> > >>> We are currently facing three alternatives for > >>> introducing 3d (x,y,z)-coordinates to the sumolib: > >>> > >>> Alternative 0 - sumolib resemble the sumo.net.xml/netconvert > >> behaviour > >>> ============================================================ > >> ========== > >>> netconvert has the ability to eat z-coordinates during the > >>> netbuilding process for z=0. > >>> > >>> This is how SUMO/netconvert is, even though I find this > annoying > >>> whenever > >>> I come along this (often enough). > >>> Here is an example, which is also part of yesterdays > >>> sumolib3d-unittest: > >>> > >>> The node-file includes 3D-coordinates: > >>> > >>> <node id="first" x="100.0" y="0.0" z="0.0" /> > >>> <node id="second" x="200.0" y="0.0" z="10.0" /> > >>> > >>> Once netconvert has finished it's work, the sumo.net.xml > contains > >>> for instance the following items: > >>> > >>> <junction id="first" type="priority" x="100.00" y="0.00" > >>> incLanes="the_other_way_0" intLanes=":first_0_0" > shape="100.00,-0.05 > >>> 100.00,-2.05 100.00,2.05 100.00,0.05"> > >>> > >>> <junction id="second" type="priority" x="200.00" y="0.00" > z="10.00" > >>> incLanes="this_way_0" intLanes=":second_0_0" > >>> shape="200.00,0.05,10.00 200.0\ > >>> 0,2.05,10.00 200.00,-2.05,10.00 200.00,-0.05,10.00"> > >>> > >>> <lane id="this_way_0" index="0" speed="13.89" length="100.50" > >>> width="2.00" shape="100.00,-1.05 200.00,-1.05,10.00"/> > >>> > >>> As you can see here, the 'first' point is pure 3d with > z=0 and > >>> results in a pure 2d 'first' junction. > >>> > >>> No so with 'second' as z is different from zero. > >>> > >>> The lane 'this_way_0' is 2.5d since it starts its shape > with a > >> 2-tuple > >>> and continues in full 3d. > >>> > >>> I can fully understand that this helpful for backward > compatibility , > >>> but it also introduces extra work, e.g. when processing > >>> sumo.net.xml files > >>> in third application. > >>> > >>> Fixing this issue in netconvert is not in my scope and > probably > >>> also not > >>> in the scope of the ticket :-) and maybe even a different > >> discussion. > >>> My conclusion about alternative 0: > >>> I believe it's normal to expect the sumolib to behave like > >>> sumo/netconvert > >>> itself, even though I don't like this behaviour. > >>> > >>> > >>> Alternative 1 (dlr-team): > >>> ========================= > >>> > >>> This is how I do understand your amendment to Ticket 2072: > >>> > >>> any geometry functions should always return 3-tuple, > regardless of > >> the > >>> dimensions of the net - also for 2d-nets. > >>> > >>> Pro: This is great for all nets that contain 3d-Nodes. > >>> Pro: This makes all users and developers of new script aware > >>> that they might consider 3d-Networks as well > >>> > >>> Con: This will break all existing scripts, once the user do > >>> update to a new version of SUMO, since up to now > >>> never 3-tuple were returned even on 3d-networks. > >>> This will result in thousands of: > >>> ValueError: too many values to unpack > >>> > >>> This will also break all strips running on 2d-networks. > >>> Even though it's perfectly fine to return 2-tuples on a > >>> 2d-Networks (Node only having x,y coords only). > >>> > >>> My personal feeling is that most Networks are in 2d. > >>> > >>> My conclusion about alternative 1: > >>> This is a lot of avoidable trouble and extra work for your > >>> users and coders, so please don't stick to it. > >>> > >>> Alternative 2 (mixture): > >>> ========================= > >>> > >>> Why not loading all nodes, and keep a record on the > net-object > >>> about its dimension. > >>> > >>> As long as there are no 3d-nodes read by startElement() > >>> the net remains as 2d net. > >>> -> net has either only 2d nodes or all 3d-nodes are in > xy-pane. > >>> > >>> Once the first node with three coords is read, the net > property > >>> 'dimension' is changed from 2 to 3. > >>> > >>> After finishing reading all nodes from xml, the > >>> nodes will be processed and missing z-coords will be > filled with > >>> z=0. > >>> > >>> If the user accesses now geometry values she/he will be > returned > >>> either 2- or 3-tuple, depending on the net he is loading. > >>> > >>> Rather then having a 2D-Wrapper (see below) i would suggest > >>> a option 'with_z=True' returning optional the missing z=0 > coord on > >>> a 2d net. > >>> > >>> btw. in python3.5 this recipe would work: > >>> x,y,z = net.getCorrds() if net.getDim() == 3 else > *net.getCoords(), 0 > >>> > >>> > >>> A word about backward compatibility with 2D wrapper > >>> ================================================ > >>> > >>> I don't see the point in having a extra function getShape2D() > >>> or getPositon2d() and so on. > >>> > >>> As already in the ticket's amendment described, it's as easy > >>> for a sumolib user to adapt the code > >>> from: > >>> x,y = getSomething() > >>> > >>> to: > >>> x,y,z = getSomething() > >>> > >>> it is more straight forward - even if z as > >>> variable is never used again > >>> > >>> I would argument that adding a wrapper whatEverFunction2D() > >>> to the whatEverFunction() might be of little effort, but > still > >>> in my opinion it will introduce dead code - But this is > at your > >>> choice. > >>> > >>> > >>> Geometries in TraCI > >>> =================== > >>> B.t.w. Traci is more strict, it returns 2d coords even > for 3d-nets. > >>> > >>> > >>> So which way should we travel? > >>> > >>> Best Marek Heinrich > >>> simoserv GmbH > >>> > >>> > >>> ------------------------------------------------------------ > >> ------------------ > >>> _______________________________________________ > >>> sumo-devel mailing list > >>> sumo-devel@lists.sourceforge.net > <mailto:sumo-devel@lists.sourceforge.net> > >>> <mailto:sumo-devel@lists.sourceforge.net > <mailto:sumo-devel@lists.sourceforge.net>> > >>> https://lists.sourceforge.net/lists/listinfo/sumo-devel > <https://lists.sourceforge.net/lists/listinfo/sumo-devel> > >>> <https://lists.sourceforge.net/lists/listinfo/sumo-devel > <https://lists.sourceforge.net/lists/listinfo/sumo-devel>> > >>> > >>> > >> > >> ------------------------------------------------------------ > >> ------------------ > >> _______________________________________________ > >> sumo-devel mailing list > >> sumo-devel@lists.sourceforge.net > <mailto:sumo-devel@lists.sourceforge.net> > >> https://lists.sourceforge.net/lists/listinfo/sumo-devel > <https://lists.sourceforge.net/lists/listinfo/sumo-devel> > >> > > > > ------------------------------------------------------------------------------ > > _______________________________________________ > > sumo-devel mailing list > > sumo-devel@lists.sourceforge.net > <mailto:sumo-devel@lists.sourceforge.net> > > https://lists.sourceforge.net/lists/listinfo/sumo-devel > <https://lists.sourceforge.net/lists/listinfo/sumo-devel> > > > > > > ------------------------------------------------------------------------------ > _______________________________________________ > sumo-devel mailing list > sumo-devel@lists.sourceforge.net > <mailto:sumo-devel@lists.sourceforge.net> > https://lists.sourceforge.net/lists/listinfo/sumo-devel > <https://lists.sourceforge.net/lists/listinfo/sumo-devel> > > ------------------------------------------------------------------------------ _______________________________________________ sumo-devel mailing list sumo-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sumo-devel
