search

njtierney / maxcovr

Tools in R to make it easier to solve the Maximal Coverage Location Problem
https://njtierney.github.io/maxcovr/
GNU General Public License v3.0
43 stars 12 forks source link

Adding new features: demand weight and distance matrix #93

Open huanfachen opened 3 years ago

huanfachen commented 3 years ago

Hi Nick. I plan to add two new features to the max_coverage function:

These features would make this package more general, which are motivated by my recent work on comparing open-source packages for location cover models. More details are here.

I would like to push these new features to this repo and would like to know your thoughts on this. Many thanks!

Best Regards,

Huanfa

mpadge commented 3 years ago

@huanfachen Much of this has already been documented in previous issues. Inclusion of distance matrices is in #77, including all code which will eventually be merged, and the weights issue is #85, which Nick and I have discussed a lot, although not much currently written in that issue. You'll also notice that both of those issues have been lying around for quite some time. Nick and I both (obviously) have other priorities preventing them from being addressed, so maybe among the most useful things for you to do would be:

  1. Pressure us to get these features incorporated and working; and even more importantly
  2. Make sure you watch this repo and help contribute to/test/modify/comment on any code that gets PR'ed in relation to the relevant issues (#77, #85).

Sound good? I guess you're in relatively frequent contact with Nick, and he knows best ways to nudge me to get us all working together on finishing these missing pieces. And finally: Great paper, and a really important contribution to open-source advocacy! Thanks for writing it, and linking here.

huanfachen commented 3 years ago

@mpadge Thanks for your comments. I have implemented a max coverage function called max_coverage_weighted that incorporates demand weight. It still needs documentation and more testing, but it is working for my cases. Hope this is something useful.

In addition, I have some suggestions on the max_coverage function:

  1. Not all applications contain a non-empty existing facility set. Therefore, this function should allow existing_facility as NA (or NULL) and skip the step of testing which users have been covered by existing facilities.
  2. The order of proposed facilities in the argument of d_proposed_facility and demand_weight (an object that contains demand weight) should be the same. This needs to be explicitly stated and/or tested in the function.

Thanks for working on this package, this is so helpful.