Skip to content

Conversation

@xaviernogueira
Copy link
Contributor

Closes #31

So I had a chance to explore options regarding IF/ELSE style process functions. The good news is nothing weird has to be added to the existing Model engine! My assessment of options (and their relative performances) is stored in examples/dev_sandbox/running_process_funcs.ipynb.

Benchmark: Simple functions (see the notebook) applied over the 25x53 xarray tutorial air temperature grid. I used %%timeit to check speeds.

Results:

  • Option 1: Using xr.where() - turns out this is quite fast (6.83ms in my last benchmark) ONLY IF you do not use the @numpy.vectorize decorator, despite that being recommended in the xarray.apply_ufunc documentation. Oddly, pre-vectorizing causes a nearly 10x slowdown (64.9ms). Also note that xr.where can not be JIT compiled with Numba.
  • Option 2: Numa JIT compiles over a loop that updates each value of a fresh numpy array one-by-one. Despite sounding a lot slower, since this is able to be JIT compiled, it is actually a tad faster (5.96ms)!

Conclusion: Since option 1 and option 2 are relatively the same speed, I may just go with option 1 since the logic is much clearer. For example, in my notebook option 1 took 2 lines, and option 2 took 7 lines.

Other changes:

  • I forgot to switch branches when adding a new test that explicitly verifies that we can update state variables in between timesteps, which is necessary for interacting with other models like Clearwater-Riverine or GSSHA.

@xaviernogueira xaviernogueira added bug Something isn't working enhancement New feature or request labels Oct 11, 2023
@xaviernogueira xaviernogueira linked an issue Oct 11, 2023 that may be closed by this pull request
2 tasks
@xaviernogueira xaviernogueira merged commit 0d7ca08 into main Oct 11, 2023
@aufdenkampe
Copy link
Member

@xaviernogueira, I just reviewed your examples/dev_sandbox/running_process_funcs.ipynb notebook and its results. It all looks quite good to me. I even tested out a few alternatives, but get basically the same results.

I agree with your decision to move forward with Option 1, because it is substantially simpler.

As an aside, I found that for the optimized function (Option 1):

  • writing to a new xarray.DataArray took about 50% the time
  • calculating combo took about 40% of the time
  • Assigning values based on xr.where() took only 10% of the time

So the selection process is very fast.

@xaviernogueira xaviernogueira mentioned this pull request Oct 11, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Figure out if/else logic for process functions

3 participants