Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Settings parcel_sizes should not influence the profit #67

Open
hanase opened this issue Dec 17, 2018 · 4 comments
Open

Settings parcel_sizes should not influence the profit #67

hanase opened this issue Dec 17, 2018 · 4 comments

Comments

@hanase
Copy link
Collaborator

hanase commented Dec 17, 2018

The feasibility model expects users to set parcel_sizes in the yaml file (default is 10000). A comment in the code claims that the value does not matter because it cancels out in the pro-forma computation. This is true for some of the pro-forma measures. However, this settings influences the construction time and hence the profit. Here is more detail.

Probably an easy fix would be to keep the construction time on the per-sqft scale and then multiply it by the real parcel size in _lookup_parking_cfg().

@jessicacamacho
Copy link
Collaborator

jessicacamacho commented Feb 14, 2019

Hi @hanase ,

Thanks for pointing this out. We have changed _lookup_parking_cfg( ) in a branch called fixes_months_calculation, so that actual parcel sizes are taken into account when calculating construction months.

@hanase, @janowicz: please take a look at https://github.com/UDST/developer/compare/fixes_months_calculation, and let me know if you have any questions or suggestions before creating a PR.

Thanks!!

Jessica

@hanase
Copy link
Collaborator Author

hanase commented Feb 19, 2019

Hi @jessicacamacho , thanks for looking into this!
It looks to me like the computation of month_values is not correct. The self.construction_months object is a matrix with columns corresponding to uses and rows corresponding to months (created here), so it's not form-specific.

@jessicacamacho
Copy link
Collaborator

jessicacamacho commented Feb 19, 2019

Hi @hanase,

Thanks for your comment. I have made a modification that should solve it, weighting the construction_months values by use (from proforma.yaml) with the percentage of each use on each form:

month_keys = self.construction_sqft_for_months
month_values = (self.forms[form] * self.construction_months).sum(axis=1)
months_dict = dict(zip(month_keys, month_values))

Please let me know if you have any additional suggestions or questions.

Thanks!

Jessica

@hanase
Copy link
Collaborator Author

hanase commented Feb 20, 2019

This works and gives the same results for different values of parcel_sizes. Thanks @jessicacamacho !
My suggestion would be to remove the need to have the parcel_sizes settings in the yaml file as it is quite confusing. Without reading the code one would not know that this setting has no impact on anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants