-
Notifications
You must be signed in to change notification settings - Fork 100
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
Dev accessibilities #614
Dev accessibilities #614
Conversation
…eeded to modify this file
|
||
# Assign persons to households | ||
rep = ( | ||
pd.DataFrame(util.named_product(hhid=households[hhid], index=persons.index)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the proto population does not allow households to have different household sizes nor different household member composition. Is that intended?
hinccat1: [1, 2, 3, 4] # Income categories | ||
hworkers: 1 # Household workers | ||
veh: [0, 1, 2] # Household vehicles | ||
persons: 2 # Two persons household |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can household size vary in the proto population? e.g. persons: [1, 2, 3]
. And can households have different composition? e.g. type 1 household has one working adult and one school child, type 2 household has one working adult and one retiree. In applications, household size and composition could have impact on mode chocie logsum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct, this was created as specified in the 'Disaggregate Accessibilities Implementation Plan' (https://github.com/ActivitySim/activitysim/wiki/Project-Meeting-2022.03.22).
You can change the household size, but you can't have multiple household sizes. E.g., you can't have a 2 person household and a 3 person household because persons must then be mapped to the households and this greatly complicates the generation procedure.
I agree that users may want this functionality, but is a bit out of scope. However, I think a simple solution for complicated cases would be to just let users to optionally create their own person/household/tour templates tables manually in a csv that gets read in. I just added this to the latest commit.
grade: 0 # Not attending | ||
timeFactorWork: 1 # mean | ||
timeFactorNonWork: 1 # mean | ||
mapped_fields: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think users would benefit from more detailed documentation on these settings in case they want to change them. E.g. when does it require users to define mappings for all vs some attributes; what is the minimum required attributes to create/map (seems like at least the ones used in the mode choice and location choice uec specs). I could be wrong but it seems there are some underlying rules like no need to define two workers in the proto pop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are person attributes specified in the plan document. However, the timeFactorNonWork and timeFactorWork are not necessary to ActivitySim and have been removed. But will mention in documentation that the proto tables must include all fields necessary to run your models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Where can I find the plan document?
sample_idx = sorted(sample_idx) | ||
elif method and method.lower() == "kmeans": | ||
# Performs a simple k-means clustering using centroid XY coordinates | ||
centroids_df = pipeline.get_table("maz_centroids") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems kmeans
will not work with one-zone system. Can we add assert
before this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added assertion for 2-zone systems. 1-zone could be implemented in future but may not be needed if aggressive sampling isn't typically requires for one-zone systems. This will require more work to ensure variable zoned centroid tables are loaded properly in tables/disaggregate_accessibility.py.
activitysim/examples/prototype_mtc_extended/configs/disaggregate_accessibility.yaml
Show resolved
Hide resolved
@nick-fournier-rsg Thank you for addressing my comments. One new question - Should at least one 2-zone example model be updated to include disagg accessibility? Right now disagg accessibility is only tested within the 1-zone MTC extended model. |
Confirmed sandag 2-zone example ran on my end, thanks for adding it. |
Draft pull request for disaggregate accessibilities. Works with 1 and 2 zone models. Tested but some issues remain: