-
Notifications
You must be signed in to change notification settings - Fork 75
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
Accelerate simulations using numba #1214
Comments
Hi @IGF-PSD and thank you for this idea! This is a very exciting potential contribution, and we would of course be very happy for performance gains 😃 In order to define the best course of action (RFC, direct implementation, level of collaboration, rollout plan…), could you share with us:
I understand this is a lot of additional information. If you prefer, we could also set up a synchronous video call to discuss these points and best understand your context 🙂 I would also love to hear from community technical experts such as @benjello @benoit-cty @bonjourmauko @Morendil @nikhilwoodruff @eraviart if they have experience, opinions or insights on using numba or alternatives on the OpenFisca codebase 🙂 |
Hello, this will be great ! |
Hello, Thank you very much for your feedback!
3-4. Integrating numba with openfisca would have a certain number of consequences for the package and its syntax:
However, we are very open to further discuss these potential changes ! |
Thank you very much for this detailed answer! A 50× compute performance gain sounds exciting 😃
Just to clarify: I understand that you extracted the code for that formula from OpenFisca (which makes total sense to get rid of the underlying non-numba classes), meaning that there was no caching. I thus assume that the formula was not recursive, as this would significantly change the assessment (i.e. maybe compute costs are insignificant compared to cache miss costs). Please let me know if that is not the case 🙂 In order to better understand the cost of upgrading and the differences in syntax, would you be able to share the two formula implementations you used for this benchmark? This would be very useful for the community to assess the differences.
If every dependent needs to rewrite every variable, this is a major drawback as the upgrade cost will be immense. To be sure: do you mean that rewriting the variables is an opt-in to enable numba optimisation on a per-variable basis, i.e. that variables as they are will still work but will not be optimised, or that once the change is brought every variable will have to be rewritten to be compatible with the numba-optimised base classes?
To my knowledge, we don't currently have any GPU optimisation in place in OpenFisca Core and have no resources secured for such an addition, so there would not be a competing implementation 🙂 |
Hi @IGF-PSD and @MattiSG! Indeed, I've been testing out improving OpenFisca with Just a concern, however: As I've been pushing mudularisation and immutability for some time, some work has already been shipped, and some other is on the WIP queue. And making code optimisible is not that hard: it is a matter of extracting from the performance bottlenecks the objects that can't be compiled natively in C/C++. So, it is above all, a design effort, pure compiler-aware refactoring. |
BTW, I know traction for typing is not huge yet within the community, yet type-hinting is just about soft-enforcing good design that allows for compiler optimisations. See, for example, a from retrievals import Page, PageParser
ctypedef object Page_t
ctypedef object PageParser_t
cdef class DocSplitter:
cpdef list[Page_t] split(self, PageParser_t parser, TextSplitter text_splitter)
cdef class TextSplitter:
cdef public int chunk_size
cdef public int chunk_overlap
cdef char* separator
cdef object separator_pattern
cpdef list[char*] split(self, char* text)
cpdef list[char*] merge(self, list[char*] splits)
cdef list[char*] get_current_chunk(self, list current_chunk)
cdef int get_current_len(self, list[char*] current_chunk) If we just enforce some of these constraints and export pure ADTs, users will be able to easily perform |
Another, probably harder up-front, but cheaper long-tail, option, that I think we discussed with @cbenz, is to generalise dependency injection of the classes that can be implemented with primitivy C types. See, for example: def calculate_add(self, variable_name: str, period):
variable: Optional[Variable]
variable = self.tax_benefit_system.get_variable(
variable_name, check_existence=True
)
if variable is None:
raise errors.VariableNotFoundError(variable_name, self.tax_benefit_system)
if period is not None and not isinstance(period, periods.Period):
period = periods.period(period)
# Check that the requested period matches definition_period
if periods.unit_weight(variable.definition_period) > periods.unit_weight(
period.unit
):
raise ValueError(
f"Unable to compute variable '{variable.name}' for period "
f"{period}: '{variable.name}' can only be computed for "
f"{variable.definition_period}-long periods. You can use the "
f"DIVIDE option to get an estimate of {variable.name}."
)
if variable.definition_period not in (
periods.DateUnit.isoformat + periods.DateUnit.isocalendar
):
raise ValueError(
f"Unable to ADD constant variable '{variable.name}' over "
f"the period {period}: eternal variables can't be summed "
"over time."
)
return sum(
self.calculate(variable_name, sub_period)
for sub_period in period.get_subperiods(variable.definition_period)
) There is no way to optimise that without extracting dependencies from the function: def guard(variable: Just[Variable], period: Just[Period]) -> TypeGuard[Match[Variable, Period]]:
# Check that the requested period matches definition_period
if Period.unit_weight(variable.definition_period) <= Period.unit_weight(period.unit):
return True
raise ValueError(
f"Unable to compute variable '{variable.name}' for period "
f"{period}: '{variable.name}' can only be computed for "
f"{variable.definition_period}-long periods. You can use the "
f"DIVIDE option to get an estimate of {variable.name}."
)
# etc...
@numba.jit(forceobj=True)
def sub_periods(variable: Just[Variable], period: Just[Period]) -> Just[Sequence[Tuple[int]]]:
return set(sub_period.value for sub_period in period.get_subperiods(variable.definition_period))
@numba.jit(nopython=True)
def calculate_add(calc: [[str, tuple], float], tuple: Just[Match[float, set[tuple[int]]]]):
return sum(map, calc, [v_value, p_values])) Something like that. |
We found that iterating certain simulations on exhaustive data using openfisca can be very slow. As openfisca relies heavily on
numpy arrays
, we thought that wrapping some of the package's classes with the numba package's jitclass decorator could greatly speed up calculations.What do you think of this idea? Would you be interested in a merge request including these changes for a future release of the package?
The text was updated successfully, but these errors were encountered: