Speed up model2netcdf.SIPNET

[infotroph]

Description

• Remove all ud_convert calls from model2netcdf.SIPNET and replace them with scalar conversions for a ~4.5x(!) speedup in netCDF writing
• Perform all unit conversions on the full input rather than separately for each year and output variable
• Use the NPP reported by Sipnet instead of recomputing it with identical results
• Fix a unit error in GWBI -- current results are too small by a factor of n_timesteps_per_day
• Avoid rereading sipnet.param each year

Motivation and Context

I reran some MAGiC ensembles using the current PEcAn develop branch and noticed a big increase in runtime -- more than half an hour for sets of simulations that used to take less than 10 minutes. As we've known for a long time, the bulk of the run time in a PEcAn run that uses Sipnet is the model2netcdf step, so I did some profiling with {profvis}:

Details

library(profvis)
devtools::load_all("models/sipnet")

# A file I happened to have handy; should work the same on any you have
sample_output <- "example/out/ENS-00001-35652c3df8ef659eacc08561b60bc811/sipnet.out"
sample_param <- "example/run/ENS-00001-35652c3df8ef659eacc08561b60bc811/sipnet.param"

# wrapper to avoid retyping setup and teardown each time I reload and test;
# `profvis(model2netcdf(...))` would work fine too
run_m2n <- function(){
  wd <- withr::local_tempdir()
  dir.create(file.path(wd, "out", "a"), recursive = TRUE)
  dir.create(file.path(wd, "run", "a"), recursive = TRUE)
  file.copy(sample_output, file.path(wd, "out", "a", "sipnet.out"))
  file.copy(sample_param, file.path(wd, "run", "a", "sipnet.param"))
  profvis(
    model2netcdf.SIPNET(
      outdir = file.path(wd, "out", "a"),
      start_date = "2016-01-01",
      end_date = "2023-12-31",
      sitelat = 1,
      sitelon = 1,
      delete.raw = FALSE,
      revision = "2.0.0"
    )
  )
}

run_m2n()
# (result opens in a browser window)

Files used above: example.tgz

When run on my machine with the current development version of model2netcdf.SIPNET, this 8-year example takes about 4-4.5 seconds.

I was expecting to see bottlenecks inside calls to ncdf4, but was surprised to see instead that it spends substantially all of the runtime inside units::ud_convert, assigning and garbage-collecting a lot of temporary objects. OK, fair, fancy unit parsing is expensive and we did just convert a lot of x/1000 conversions to ud_convert(x, "g/m2", "kg/m2") in #3719, which lines up with the timing of this regression.

I converted the ud_convert calls back to scalar conversions, and additionally switched from repeated conversions for every output calculation in every year to one single pass through the raw output file before the start of the year loop. The result runs in less than 1 second with no GC pauses and most of the remaining time spent in netCDF setup.

[mdietze]

FWIW that remaining time spent on datetime2cf also involves a call to ud.convert under the hood, in case you're looking for one more speed up

also please make sure the changed assumption about reading params is well documented, because that's going to come back to bite us when we do parameter updating (e.g., crop planting or integrating iterative parameter calibration into the SDA)

[infotroph]

please make sure the changed assumption about reading params is well documented

@mdietze Which change do you mean here? Note that this PR does not implement the change to get LAI from the output file that we're currently discussing in Slack -- Here all I did was switch from rereading the parameter file for each year of output to noting that it won't have changed between years and reading it once per call.

[infotroph]

FWIW that remaining time spent on datetime2cf also involves a call to ud.convert under the hood, in case you're looking for one more speed up

I noticed that but figured I should stop while I'm ahead rather than get stuck rediscovering why all the time conversion corner cases really do matter 🤣

[infotroph]

Failing CI jobs appear to be a GitHub API issue. I'll wait a few hours and try rerunning.

[dlebauer]

Avoiding repeated ud_convert makes sense and it is great that it provides such a speedup. But ud_convert is still very valuable in its ability to ensure correctness.

For these conversions, it seems it would be possible to retain both the speedup and correctness using ud_convert to define constants once near the top of the file e.g. cm_to_mm <- ud_convert(1, 'cm', 'mm')?

It also sets better precedent, since it is likely that this same approach would speed up code elsewhere in the codebase.

On the topic of speedup, while reviewing this I recalled that there are a lot of calls in the PEcAn version of ud_convert (ud_are_convertible, as_units, set_units, drop_units) while the units package directly calls a C function. So I checked out the penalty, and found that the units implementation is about 20x faster and uses infinitely less memory.

bench::mark(
  pecan = PEcAn.utils::ud_convert(42, "m", "km"),
  units = units::ud_convert(42, "m", "km"),
  iterations = 500
)
#> # A tibble: 2 × 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 pecan      142.35µs 151.56µs     6445.    6.44MB     52.0
#> 2 units        6.56µs   7.58µs   114353.        0B    229.