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

Adding fixed-origin to fixed-primary-destination skim wrapper #606

Merged
merged 10 commits into from
Jan 3, 2023
15 changes: 15 additions & 0 deletions activitysim/abm/models/trip_destination.py
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,7 @@ def sample_skims(self, presample):

o = self.model_settings["TRIP_ORIGIN"]
d = self.model_settings["ALT_DEST_COL_NAME"]
n = self.model_settings.get("PRIMARY_ORIGIN", "origin")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The structure of this change is simple enough, but our lack of a cohesive unit testing regime means there's no "test" that this works without also committing the model configs that it is supposed to go with. We'll need to discuss whether we can make changes like this without proper testing.

p = self.model_settings["PRIMARY_DEST"]

if presample:
Expand All @@ -935,6 +936,8 @@ def sample_skims(self, presample):
skims = {
"od_skims": skim_dict.wrap(o, d),
"dp_skims": skim_dict.wrap(d, p),
"op_skims": skim_dict.wrap(o, p),
"nd_skims": skim_dict.wrap(n, d),
"odt_skims": skim_dict.wrap_3d(
orig_key=o, dest_key=d, dim3_key="trip_period"
),
Expand All @@ -947,6 +950,18 @@ def sample_skims(self, presample):
"pdt_skims": skim_dict.wrap_3d(
orig_key=p, dest_key=d, dim3_key="trip_period"
),
"opt_skims": skim_dict.wrap_3d(
orig_key=o, dest_key=p, dim3_key="trip_period"
),
"pot_skims": skim_dict.wrap_3d(
orig_key=p, dest_key=o, dim3_key="trip_period"
),
"ndt_skims": skim_dict.wrap_3d(
orig_key=n, dest_key=d, dim3_key="trip_period"
),
"dnt_skims": skim_dict.wrap_3d(
orig_key=d, dest_key=n, dim3_key="trip_period"
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks probably fine for "legacy" use, but it's going to break the sharrow integration. There's a parallel data structure for sharrow defined here that will need to be updated if this will work with sharrow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed this breaks the sharrow integration.
14/12/2022 11:34:11 - ERROR - activitysim.core.flow - error in apply_flow: name 'op_skims' is not defined

  File "c:\users\wangs1\workspace\activitysim\activitysim\core\interaction_sample_simulate.py", line 161, in _interaction_sample_simulate
    ) = interaction_simulate.eval_interaction_utilities(
  File "c:\users\wangs1\workspace\activitysim\activitysim\core\interaction_simulate.py", line 203, in eval_interaction_utilities
    sh_util, sh_flow = apply_flow(
  File "c:\users\wangs1\workspace\activitysim\activitysim\core\flow.py", line 774, in apply_flow
    flow_result = flow.dot(
  File "c:\users\wangs1\workspace\sharrow\sharrow\flows.py", line 1784, in dot
    return self._load(
  File "c:\users\wangs1\workspace\sharrow\sharrow\flows.py", line 1562, in _load
    result = self.iload_raw(source, runner=runner, dtype=dtype, dot=dot)
  File "c:\users\wangs1\workspace\sharrow\sharrow\flows.py", line 1476, in iload_raw
    raise NameError(problem.group(1)) from err
NameError: name 'op_skims' is not defined

}

return skims
Expand Down