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

Utility terms that compare pandas categorical variable to strings are not evaluated correctly with Sharrow #766

Closed
i-am-sijia opened this issue Dec 15, 2023 · 3 comments
Assignees
Labels
Bug Something isn't working/bug f

Comments

@i-am-sijia
Copy link
Contributor

Describe the bug
After implementing the string to pandas categorical conversion, some of our current CI tests failed. They all had Sharrow turned on and set to test mode. The utility calculated with and without Sharrow are different.

[48:39.10] INFO: completed flow_LQLDEWSFEGQ5W2NJNONCWPNFSCB7O5RD.load in 0:00:18.710844 stop_frequency.work.simple_simulate.eval_mnl
[48:39.10] INFO: completed apply_flow in 0:00:21.651810 
[48:39.10] INFO: elapsed time sharrow flow 0:00:21.659376 stop_frequency.work.simple_simulate.eval_mnl
[48:39.31] INFO: elapsed time simple flow 0:00:00.207622 stop_frequency.work.simple_simulate.eval_mnl.eval_utils

Not equal to tolerance rtol=0.01, atol=0
utility not aligned
Mismatched elements: 132 / 144 (91.7%)
Max absolute difference: 1998.00011762
Max relative difference: 1729.2712081
 x: array([[    0.     , -1000.9582 , -1002.2882 , -1002.6522 , -1001.3462 ,
        -2000.7913 , -2002.1212 , -2002.4852 , -1003.1262 , -2002.5713 ,
        -2003.9012 , -2003.5703 , -1004.4472 , -2003.8922 , -2004.5272 ,...
 y: array([[ 0.000000e+00, -1.958200e+00, -3.288200e+00, -3.652200e+00,
        -2.346200e+00, -2.791200e+00, -4.121200e+00, -4.485200e+00,
        -4.126200e+00, -4.571200e+00, -5.901200e+00, -5.570200e+00,...
big problem: 132 missed close values out of 144 (91.67%)
sh_util.shape=(9, 16)
(array([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1,
       1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
       2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 4,
       4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 6,
       6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 7, 7, 7, 7, 7, 7, 7, 7,
       7, 7, 7, 7, 7, 7, 7, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8]), array([ 1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,  1,  2,
        3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,  1,  2,  3,  4,
        5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,  1,  2,  3,  5,  6,  7,
        9, 10, 11, 13, 14, 15,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11,
       12, 13, 14, 15,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13,
       14, 15,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
        1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,  1,  2,
        3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15]))
possible problematic expressions:
  11.1% [043] (school_esc_outbound.isin(['ride_share', 'pure_escort']))
  00.0% [044] (school_esc_inbound.isin(['ride_share', 'pure_escort']))
[48:43.24] ERROR: ===== ERROR IN stop_frequency =====
[48:43.24] ERROR: 
Not equal to tolerance rtol=0.01, atol=0
utility not aligned

To Reproduce
Steps to reproduce the behavior:

  1. Check out ...
  2. Run test_mtc_extended.py::test_prototype_mtc_extended_sharrow()

Expected behavior
The utility with and with Sharrow should be the same.

Screenshots
result of tracing the failed tour, in the stop_frequency.work:
Chooser
image

Non-sharrow evaluation
image

Sharrow evaluation]
image

Additional context
Temporary solution: I moved the pandas categorical vs string comparisons to the preprocessors.

@i-am-sijia i-am-sijia added the Bug Something isn't working/bug f label Dec 15, 2023
@jpn-- jpn-- moved this to Todo in Phase 9 Work Feb 5, 2024
@jpn-- jpn-- self-assigned this Feb 5, 2024
@jpn--
Copy link
Member

jpn-- commented Apr 2, 2024

This should be addressed by the recent fix applied to sharrow and included in sharrow version 2.8. There are included in that work unit testing to ensure that categoricals are being properly handled.

@i-am-sijia are you able to easily test this now works correctly within ActivitySim?

@jpn-- jpn-- moved this from Todo to Pending Review in Phase 9 Work Apr 2, 2024
@i-am-sijia
Copy link
Contributor Author

@i-am-sijia are you able to easily test this now works correctly within ActivitySim?

I will test it.

@i-am-sijia
Copy link
Contributor Author

@i-am-sijia are you able to easily test this now works correctly within ActivitySim?

I will test it.

Confirmed I was able to recreate the error with the old sharrow, not with the new sharrow. So the bug seems to be fixed.

@github-project-automation github-project-automation bot moved this from Pending Review to Done in Phase 9 Work Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working/bug f
Projects
Status: Done
Development

No branches or pull requests

2 participants