You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
# FIXME setting index as "Alt" causes crash in estimation mode...
# happens in joint_tour_frequency_composition too!
# alts = simulate.read_model_alts(state, model_settings.ALTS, set_index="Alt")
alts = simulate.read_model_alts(state, model_settings.ALTS, set_index=None)
alts.index = alts["Alt"].values
I noticed all other models' alternatives files have the alternative column named and hardcoded in source code as alt, but for school escorting, it is Alt. It's probably better to keep the column name consistent as alt, even though we are not addressing the "hardcoded" part at this moment.
I think we should treat this issue separately from this PR. If we change it here, it will have implications for people who are already using the model (i.e. they would have to change their alternatives file). This is larger than simply changing it here as the Alt name is coded in a couple of places in this file and I would like to keep this PR focused on the BayDAG contributions. Thus, I think we should track it in a separate and stand-alone issue. Thoughts?
I noticed all other models' alternatives files have the alternative column named and hardcoded in source code as
alt
, but for school escorting, it isAlt
. It's probably better to keep the column name consistent asalt
, even though we are not addressing the "hardcoded" part at this moment.Originally posted by @i-am-sijia in #777 (comment)
I think we should treat this issue separately from this PR. If we change it here, it will have implications for people who are already using the model (i.e. they would have to change their alternatives file). This is larger than simply changing it here as the Alt name is coded in a couple of places in this file and I would like to keep this PR focused on the BayDAG contributions. Thus, I think we should track it in a separate and stand-alone issue. Thoughts?
Originally posted by @dhensle in #777 (comment)
The text was updated successfully, but these errors were encountered: