EcohydrologyTeam / ClearWater-riverine

A 2D water quality transporter model to calculate conservative advection and diffusion of constituents from an unstructured grid of flows
MIT License
6 stars 0 forks source link

Speed up Summing Coefficients to Diffusion Term #58

Closed sjordan29 closed 6 months ago

sjordan29 commented 7 months ago

Closes #49

This PR increases the speed of model instantiation through the following changes:

  1. Eliminating the need for inefficient _calc_sum_coeff_to_diffusion_term, which included a loop over time. Rather than calculating this value during model instantiation, it is now calculated within the LHS class's update_values method, taking advantage of the csr_matrix which tracks which values need to be summed in each position of the LHS matrix for us. Moving the calculation here does not impact model runtime.
  2. While profiling the code, also found there were several list comprehension steps that convert the binary datetimes to useable datetimes that were extremely slow. Tried a few options and found that leveraging pandas worked best for this use case. Additional speed gains may be possible, but this was a quick and simple way to make gains.
  3. Together, these changes dropped model instantiation time more than 20x for models with many timesteps (i.e., the 3-day simulation at one-second timestep).

Next steps to improve speed should focus on

aufdenkampe commented 6 months ago

@sjordan29, thanks for these important speedups!