pgr_gsoc_vrppdtw rename this function to something reasonable

[cvvergara]

Related to #439

[cvvergara]

here are some suggestions for names instead of pgr_gsoc_vrppdtw: pgr_vrpPickDelivery pgr_PickDeliveryVRP pgr_PickDelivery pgr_vrpPickupDelivery pgr_PickupDeliveryVRP pgr_PickupDelivery

[cvvergara]

I finished the code of pick and delivery that fixes several issues, like leaks, server crash. No one gave an opinion on the name I named it pgr_pickDeliver I want to merge to develop, but I need to know if:

1. The name I selected is OK.
2. Is backwards compatability with pgr_gsoc_vrppdtw necessary? I must mention that pgr_gsoc_vrppdtw is proposed function and also pgr_PickDeliver will be proposed function. @dkastl @woodbri @robe2 I really need a comment on this so when I merge to develop I
3. have a reasonable name
4. know if I need to keep backwards compatability. So I can do the appropriate changes

[dkastl]

1. The name I selected is OK.

pgr_pickDeliver? I would say it's OK. Doesn't sound great, but I have no better idea.

2. Is backwards compatability with pgr_gsoc_vrppdtw necessary?

In my opinion "no", because it was so far release as experimental or so.

[woodbri]

If we think we will have various vrp related functions over time then I would suggest using something like vrp_pickupDeliver then other function can be group under the vrp_* prefix.

I don't think you need to worry about backwards compatibility.

I like using the name timeMatrix to make it clear. And you only need to worry about average speed if you are giving locations and want a Euclidean timeMatrix build. In fact, I would make a generator for the the Euclidean timeMatrix so you can do something like:

select * from vrp_pickupDelivery('select * from euclideanTimeMatrix(''select * from locations'', average_speed)', ...);

This make the timeMatrix generation reusable by other function in the future.

[cvvergara]

so thinking on the future.... similar to tsp:

pgr_eucledianTSP calculates the cost matrix on the fly. saves memory. pgr_TSP recives a cost matrix

then pgr_eucledianPickDeliver <--- is this one, calculates on the fly... pgr_pickDeliver <--- this one does not exist but would receive a time matrix

So I think a better name is pgr_eucledianPickDeliver to start a kind of standard

[dkastl]

I like Steve's idea with the "vrp_" prefix.

[cvvergara]

I dont like Steve's idea of vrp prefix pgrouting's prefix is pgr. and in the index of the documentation is just a matter to make a section for vehicle routing problems. also a suffix caould be used

[cvvergara]

Anyway, as it is proposed:

• the name can change later
• functionality could change
• parameters could change... etc

We can leave the name "pgr_gsoc_vrppdtw" and decide later. but I'll still add the speed parameter to convert the distances to time

[cvvergara]

Following the last comment it would mean that name remains the same, but parameters changes & results change, and improved solutions.

[cvvergara]

lots of options, lets wait for @robe2 's opinion, and then we can vote with thombs up or down

[cvvergara]

For reference: this is how the data should look: (the table matches the customers inner query columns names) https://github.com/cvvergara/pgrouting/blob/pickDeliver/pgr_pickDeliverClass/src/pickDeliver/test/pickDeliver.data

this is how results look keeping the backwards compatability with pgr_gsoc_vrppdtw: https://github.com/cvvergara/pgrouting/blob/pickDeliver/pgr_pickDeliverClass/src/pickDeliver/doc/gsoc_vrppdtw.queries

This is how results look lwith the changes I made: https://github.com/cvvergara/pgrouting/blob/pickDeliver/pgr_pickDeliverClass/src/pickDeliver/doc/doc-pickDeliver.queries

[cvvergara]

As you can see (in the first link) the coordinates are given in the customers's data I dont even remember what is: etime integer, ltime integer, stime integer So maybe the inner query column names might need a change.

[woodbri]

On 7/1/2016 12:07 AM, Vicky Vergara wrote:

As you can see (in the first link) the coordinates are given in the order's data I dont even remember what is: etime integer, ltime integer, stime integer So maybe the inner query column names might need a change.

Yeah names should change to something meaningful for the window times like:

open_time or start_time close_time or end_time service_time

Or something along those ideas.

---

This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus

[woodbri]

postgis currently has 3 packages that it installs, postgis, topology and raster. For me, It is not an issue of documentation as I don't always have the available, but when I look at the function list is pgadmin to get the correct spelling and/or parameters, I find it nice that the functions are grouped by the prefix and functionality. If I'm look for a routing function pgr_* make sense, if I'm looking for a vrp function vrp_* makes sense. I could also see splitting routing functions and vrp function into two packages in the future if the number of function gets unwieldy over time. Minimizing function propagation or at least grouping functions has some value in finding any particular functionality so I would prefer names _ where might be dijkstra or pickupdeliver or etc. and might Euclidean or WithPoints etc. This way all dijkstra* functions are group together when listed.

[robe2]

Haven't read thru the thread - but I'd go with name pgr_pickupDelivery. Definitely something with pgr in it, since they all float together and you know it's a pgRouting function.

I'm torn with having vpr in name. I kind of like that allah tsp_ ... type functions so you know it's a particular kind of problem you are solving. Then again I hate a bunch of underscores. So if you need vrp, I'd say make it pgr_vrpPickupDelivery.

[cvvergara]

changed the inner query column names, added the speed, to calculate the correct travel_time I changed the function's name to: _pgr_pickDeliver <---- notice the underscore I want to merge to develop, and when things are in develop: the links in the naming issue will point to main repo the documentation I made/modified for the 2 functions (_pgr_pickDeliver & pgr_gsoc_vrppdtw) will be in develop documentation With more information a better decision can be made. I think.

[manikanta-kondeti]

vrpPickDeliver makes more sense than simple pickDeliver. As a dev, I would prefer understanding the context of the method through its name. Just my opinion, we can ignore it 💃

[woodbri]

I think it makes sense to group all the bro functions together using something like vrp_ Vicky has alway made the point that function names should be descriptive and it seems that we should use the same on user facing functions. My two cents, but I’m not doing this so the cooks get to make the decision 😀

Sent from my iPhone

On Jun 5, 2018, at 7:08 AM, Manikanta [email protected] wrote:

vrpPickDeliver makes more sense than simple pickDeliver. As a dev, I would prefer understanding the context of the method through its name. Just my opinion, we can ignore it 💃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.