Skip to content

Conversation

@SouthEndMusic
Copy link
Contributor

@SouthEndMusic SouthEndMusic commented Nov 3, 2025

Issue addressed

Fixes #326

Explanation

My apologies for the bloated PR. Here's an overview of what I did.

Units

  • units.jl is a new script which describes unit behavior. It defines the Unit object, and as well as:
    • Functions to create the Unit object
    • Functions to translate values to and from SI units
    • Functions to represent units either in a pretty way or in the BMI standard
    • Functions for basic operations on units: multiplication, exponentiation
    • A function to get the unit of a variable from its standard name
  • The unit of variables are denoted in what was standard_name.jl, which is now subdivided into 4 files as the amount of data in this file was getting quite large:
    • standard_name_sbm.jl
    • standard_name_domain.jl
    • standard_name_routing.jl
    • standard_name_sediment.jl
  • Input data is converted to SI units as close as possible to where they are read from the input files, in the following functions:
    • get_at
    • set_states!
    • ncread
  • All internal computations are now done in SI units as much as possible, and I annotated the computations with the corresponding units. Some computations I left in the original units, namely the empirically derived ones which contain (a lot of) magical constants (e.g. rainfall_erosion_eurosem).
  • Results are converted to their expected unit when writing the results in the following functions:
    • write_csv_row
    • TODO: write_netcdf_timestep
  • BMI.get_var_units now returns SI units, as the internal arrays which are accessed through the BMI are expressed in SI units

The timestep unit made this PR quite tricky. That is because in many places rates (something per timestep) was implicitly converted to amounts (something), which was possible because the conversion factor is then 1, but it makes for hard to understand code unit-wise.

Others

  • I improved (in my opinion) the formatting of the @info messages for initialized data with prettytables.jl. See the function to_table. Here's an example:
┌ Info: Set parameter using netCDF variable as cyclic parameter.
│ ┌─────────────────┬─────────────────────────────┐
│ │ option          │ value                       │
│ ├─────────────────┼─────────────────────────────┤
│ │ parameter       │ vegetation__leaf_area_index │
│ │ netCDF_variable │ LAI                         │
│ │ n_timesteps     │ 12                          │
│ │ unit            │ m² m⁻²                      │
└ └─────────────────┴─────────────────────────────┘
  • I removed quite some redundant constructors, namely those which depend only on an amount n. These can easily be replaced by the constructors generated by @with_kw
  • I renamed all variables named amount in the sediment module into more descriptive ones (soil_erosion_rate, sediment_rate, sediment_transport_capacity)
  • I considered adding the unit to InputEntry for IO convenience, but I ultimately decided against it. During this process I did come up with a simplified interface to modify to config:
# old
config.input.forcing["atmosphere_water__precipitation_volume_flux"] =
Wflow.init_config_section(
    Wflow.InputEntry,
    Dict("scale" => 2.0, "netcdf_variable_name" => "precip"),
)

# new
config.input.forcing["atmosphere_water__precipitation_volume_flux"] =
    Dict("scale" => 2.0, "netcdf_variable_name" => "precip")

Final notes

  • The main reason a custom implementation was made instead of using e.g. Unitful.jl is to handle having the timestep as a unit, which is model dependent.

Checklist

  • Updated tests or added new tests
  • Branch is up to date with master
  • Tests & pre-commit hooks pass
  • Updated documentation if needed
  • Updated changelog.qmd if needed

@SouthEndMusic SouthEndMusic marked this pull request as draft November 3, 2025 15:09
@SouthEndMusic SouthEndMusic changed the title first draft Use SI units for internal computations Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review units of variables

2 participants