search

ActivitySim / sandag-abm3-example

BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

Ambiguous spec expressions #14

Open jpn-- opened 2 months ago

jpn-- commented 2 months ago

In the joint tour frequency and composition component, we have (for example):

https://github.com/ActivitySim/sandag-abm3-example/blob/85c1ecb4e17ad63a87e84aadf56a26d28e1c1488/configs/resident/joint_tour_frequency_composition.csv#L56

This expression is summarized as (bool * bool) + (bool * bool). The two parenthetical terms each neatly and correctly resolves to a binary value regardless of whether the operands are treated as literal boolean values or their (0,1) numerical equivalent. However, + operator is not so clean; if both operands are True, we could arrive at different results:

  1. Interpret as numeric, so1 + 1 = 2, or
  2. Interpret as boolean, so True + True = True.

The numexpr engine of pandas.eval will (with arguably good reason) punt on solving this, throwing a NotImplementedError. Pandas can fall back to numpy logic, which will solve the expression based on the logic (2) and get True. Sharrow converts the booleans to numbers, using logic (1).

It would be better to write expressions so they are less ambiguous, and (obviously) so they resolve the same with or without sharrow. Based on context clues from the rest of the spec, it appears the intention of these expressions is following logic (1). @dhensle can you (or whomever at RSG crafted this spec) confirm the preferred interpretation?

i-am-sijia commented 2 months ago

I can answer this since I worked on the initial conversion of joint tour frequency composition model.

This is what's coded in the BayDAG CT-RAMP UEC for the joint tour frequency composition model:

image

In CT-RAMP, expressions are returned in numeric. A==B returns either numeric 1 or 0, which means this expression should return 0, 1, or 2.

So this expression should be rewritten to follow logic (1).

@dhensle do you agree? If we correct the expressions, it will cause abm3 results to change in the sharrow off mode, perhaps slightly, depending on how many 2+ joint tour households are there.

dhensle commented 2 months ago

Yes, agreed. We should be replicating the original CT-RAMP logic/results.

cc: @aletzdy