prasadtalasila / TransportScheduler

concurrent implementation of CSA algorithm using cell model
GNU General Public License v3.0
2 stars 5 forks source link

new station module #58

Closed prasadtalasila closed 6 years ago

prasadtalasila commented 7 years ago

The new station module needs to be created with workex + gen_server + fsm. Obviously, the newly created station module must pass all the unit tests written so far.

SuhasHebbar commented 6 years ago

Workex as I can see in https://github.com/sasa1977/workex is not an addon functionality like fsm. It instead is implemented as an independent module with its own start_link and init functions. There doesn't seem to be a way to merge them. But in the implementation of workex + fsm, the easily queryable genserver state functionality is lost ( workex doesn't offer an inbuilt way to query the internal state of the worker process).

SuhasHebbar commented 6 years ago

We have previously discussed of how the overfilling of the Station Processes' message queues could be the possible reason of the memory "explosion" for some queries. For this we had planned to use workex (or Flow or GenStage) for aggregate operations to handle this problem.

I looked into possible methods to put an arbitrary hard cap on a processes' message queue to see if that can give any improvements but there seems to be no easy way to do so, hence any optimisation would have to be in the algorithm itself.

prasadtalasila commented 6 years ago

@SuhasHebbar please complete the work on PR #64. Once that is done, lets complete the profiling of the resulting code. After that we can take a call on finding the right location to optimise (algorithm / implementation)

SuhasHebbar commented 6 years ago

Presently the itinerary is a list consisting the vehicle links between stations with the query at the head. Do you think it would improve code comprehensibility if it is instead represented as a tuple of query and the itinerary?

Then instead of [ query, link1, link2, .... , link n ] we would have { query, [link1, link2, ...... , linkn] }. It would only be a minor change.

Also a suggestion for minor optimisation to be consider after PR#64 Presently every time a station receives a query it constructs a map of neighbouring stations. Unless the update function is called the list of neighbouring stations remains unchanged hence, it would make more sense to create the map during initialisation and after every update instead.

prasadtalasila commented 6 years ago

@SuhasHebbar please point to the sections of the code in master and ts-v1.0-alpha branches that uses [ query, link1, link2, .... , link n ] list. Are there any coding improvements possible with changing the list to a tuple?

As far as the constructing the map of the neighbours goes, again please provide links to sections of the code doing this. Obviously, your suggestion of constructing only on updates makes perfect sense.

SuhasHebbar commented 6 years ago

The assumption of [ query, link1, link2, .... , link n ] is pretty implicit and has been carried over from the master branch.

The implicit assumption made can easily be missed by someone who is not familiar with the project. https://github.com/SuhasHebbar/TransportScheduler/blob/45fa9b045fcc330b6bfea4d862da6d893590a68f/lib/ts/station_fsm.ex#L13

https://github.com/SuhasHebbar/TransportScheduler/blob/45fa9b045fcc330b6bfea4d862da6d893590a68f/lib/ts/station_fsm.ex#L137-L139

And the following shows how the links are added at the end after the query element. https://github.com/SuhasHebbar/TransportScheduler/blob/45fa9b045fcc330b6bfea4d862da6d893590a68f/lib/ts/station_fsm.ex#L174

prasadtalasila commented 6 years ago

Instead of just a tuple, having a structure for query would be good. I remember discussing the structure last semester. The required elements of the structure are: query, preferences, itinerary, cost.

Please propose the query structure before you put it into the code.

prasadtalasila commented 6 years ago

Worked completed in PR #78.