-
Notifications
You must be signed in to change notification settings - Fork 118
Common settings for fastmath, sharrow_skip, and other compute controls across components #824
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
Conversation
|
Just wanted to make sure you knew that the string removal work was performed already here: ActivitySim/activitysim-prototype-mtc#4 |
dhensle
left a comment
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.
There are a number of submodels which do not include this setting, e.g. destination models, scheduling, transit pass subsidy, etc. Those also need to be passed with the fastmath argument, especially since their yaml files will now accept the setting since the BaseLogitComponentSettings was updated.
also forbid extra in pydantic settings
|
@dhensle you make a good point. Also, while I'm pushing fastmath through all the components, I realized its convenient to wrap all component level sharrow controls together (i.e., also |
…rt-sharrow # Conflicts: # activitysim/core/interaction_simulate.py
…rt-sharrow # Conflicts: # activitysim/abm/models/school_escorting.py
|
@dhensle this PR is ready for your re-review. I believe I addressed your concerns, and then some. |
|
In thinking about the solution for #842, I realized we probably want these controls to be more general that "sharrow_settings", something more like "compute_settings". The reason we are seeing the overflow in #842 seems to be because we are using bottleneck for the computations, but if we activate |
dhensle
left a comment
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 you just missed the transit_pass_subsidy model. Other than that I think it looks good.
The reason why the school escort model was failing with sharrow relates to the
fastmathsetting.Part of how sharrow accelerates code evaluation is by turning on the "fastmath" compiler optimizations in Numba. Using this setting has numerous effects, but two are salient here:
NaN, e.g. normally evaluatingx < 0should result inFalsewhenxisNaNbut withfastmathturned on the result might beTrueinstead; the actual result of such an operation is not defined and might be hardware dependent.We do not want to simply turn off
fastmath, as the speed we get from (1) is desirable and the problems created by (2) don't actually occur in most models, as most data columns in our tables don't include missing values. But the school escort model DOES include missing values (e.g. the age of the 3rd child in a household with less than 3 children).This PR introduces a
sharrow_fastmathsetting for all relevant components, withTrueas the default for nearly all Sharrow-compiled model components, but adds a mechanism to turn it off for individual components as needed. The school escort model has the default for this setting changed toFalse.