UDST / developer

Redesigned UrbanSim developer/pro forma models
https://udst.github.io/developer/
BSD 3-Clause "New" or "Revised" License
3 stars 5 forks source link

Keeping less profitable proposals #64

Closed janowicz closed 6 years ago

janowicz commented 6 years ago

This PR introduces changes to optionally retain less profitable proforma proposals and to support proposal selection when there can be multiple proposals per parcel.

bridwell commented 6 years ago

It looked like in previous commits (in the nbest branch) you were testing the code I provided a few months back that attempted to address this issue, while also simultaneously satisfying multiple demand segments (i.e. running a single developer model with target units to build by building type). Did you find issues in that code? Or is that just beyond the scope of this PR?

janowicz commented 6 years ago

@bridwell I wanted to make sure that the functions you wrote would be mostly compatible with the changes we were making here to proforma and developer. I think that with the changes here (particularly having developer working with feasibility in long-form), you'll be able to supply your intra-parcel picking function (or a modified version of it) as a custom_selection_func. I also wanted you to be able to submit your own PR with those functions (if you would like). It worked well when I tested it. Like I mention above, I'll move proposal selection functions to their own module, and this could be a good place for the intra-parcel selection logic you implemented. And then the user will have a set of functions to choose from to plug into pick, depending on their needs. We may also need to allow target_units parameter to accept a dictionary or Series (the demand variable in your code).

bridwell commented 6 years ago

Sounds good. Thanks.

janowicz commented 6 years ago

No prob. The focus of this PR was really on keeping less profitable proposals for a given form in sqftproforma.py, and having less profitable forms/proposals not be dropped in developer.py. There was comparatively less attention paid to the proposal selection logic, but hopefully the above helps pave the way for easier incorporation of the functions you worked on regarding intra-parcel proposal picking satisfying multiple demand segments.

janowicz commented 6 years ago

@Eh2406 The default proposal selection approach when there are multiple proposals per parcel is now weighted random choice, in line with what we were using before in the one-proposal-per-parcel case, except with added logic for avoiding multiple proposals being selected for the same parcel. Also moved the proposal selection logic to functions in their own module (others can be added here).

janowicz commented 6 years ago

@bridwell Just inventorying steps that could be taken as part of a future PR to finish making the developer module compatible with your intra-parcel proposal picking functions.

And then, if you had a feasibility table in long-form from sqftproforma, the function you coded could be the default way of picking proposals when multiple_demand_segments is True. Or you could wrap your function and supply it as a callback, something perhaps like the following:

def multidemand_pick(developer, proposals, profit, target_units):
    proposals['max'] = profit
    return iter_pick(proposals.sort_values('w1', ascending=False), target_units)

dev.pick(custom_selection_func=multidemand_pick)