Ribasim icon indicating copy to clipboard operation
Ribasim copied to clipboard

Replace (time, node_id) order by (node_id, time)

Open visr opened this issue 2 years ago • 0 comments

I thought I fixed the sorting issue by updating the sorting functions as described in https://github.com/Deltares/Ribasim/pull/597#issuecomment-1719792565.

But this fixed sorting actually exposed as issue fixed in 8bc58952ec5dfe0e4f5efa6d9060c529959be660. That now filters the time tables to create contiguous vectors with the timeseries per node. For this access pattern it would be faster to have it first sorted on node_id, and then on time, instead of the other way around.

I initially went for time first since we were reading in time series at runtime, and then loading them in for all nodes. But now we create an interpolation object per node, that takes a vector of the entire timeseries. So it's better to sort by node_id first like all other tables.

  • [ ] update documented sort order in usage.qmd #903
  • [ ] update sort methods in python #903
  • [ ] update sort methods in julia
  • [ ] address TODO in 8bc58952ec5dfe0e4f5efa6d9060c529959be660 and use views like below
  • [ ] use interpolation objects instead of update_basin, which still relies on time being sorted first
  • [ ] use interpolation objects (which already seem to exist) instead of update_tabulated_rating_curve!, which still relies on time being sorted first
rows = searchsorted(time.node_id, node_id)
time_id = view(time, rows)

visr avatar Sep 14 '23 17:09 visr