diff --git a/rubem/hydrological_processes/_surface_runoff.py b/rubem/hydrological_processes/_surface_runoff.py index ebd4fe1f..3ff862df 100644 --- a/rubem/hydrological_processes/_surface_runoff.py +++ b/rubem/hydrological_processes/_surface_runoff.py @@ -39,10 +39,8 @@ def get_coef_soil_moist_conditions( :returns: Coefficient representing soil moisture conditions [-] :rtype: Field ``PCRASTER_VALUESCALE=VS_SCALAR`` """ - tur = actual_soil_moist_cont / ( - soil_bulk_density * rootzone_depth * 10 - ) # [%] soil moisture - return (tur / soil_moist_cont_sat_point) ** beta + + return (actual_soil_moist_cont / soil_moist_cont_sat_point) ** beta @staticmethod def get_runoff_coef_permeable_areas( diff --git a/tests/fixtures/AUDIT.md b/tests/fixtures/AUDIT.md index c54afe83..d727c8f2 100644 --- a/tests/fixtures/AUDIT.md +++ b/tests/fixtures/AUDIT.md @@ -1,6 +1,88 @@ # Golden fixture audit -## Why the goldens changed +## 2026-08: the Ch soil moisture unit correction (partial regeneration) + +`SurfaceRunoff.get_coef_soil_moist_conditions` converted the actual soil +moisture content from mm to a fraction (`Tur / (Dg * Zr * 10)`) before +dividing it by the saturation point. That conversion was wrong: the caller +already passes `soil_moist_content_sat_point` in mm (`_dynamic_model.py` +builds it as `tusat * Dg * Zr * 10`), so Ch compared a fraction against a +depth and came out smaller than it should be by a factor of +`(Dg * Zr * 10) ** beta` — with this fixture's `Dg` (1.25-1.64), `Zr` +(22-92 cm) and `beta` (0.5), a factor of roughly 24 to 39. Ch multiplies +`(P - I)` directly in `get_surface_runoff`, with no clipping, so the surface +runoff was suppressed by that factor. The division was removed and Ch is now +the mm/mm ratio `(Tur / Tursat) ** beta`. + +Only the 12 goldens whose values actually moved were replaced; the other 33 +keep the bytes of the previous regeneration, so the diff of this change is +exactly the affected variables: + +- `srn`, `smc`, `rnf`, `arn` at timestep 2, as `.002` map, `.tif` and + `tss_*.csv`. + +Timestep 1 is untouched because the fixture sets `t_ini = 1.0`, so the +initial soil moisture equals the saturation point and `get_surface_runoff` +takes its saturated branch (`srn = P - I`), which does not read Ch. The +variables that do not depend on Ch (`itp`, `bfw`, `eta`, `lfw`, `rec`) are +bit-identical. + +### Generating environment of the replaced files + +- Base commit: branch `fix_318` (`eab4553`, the Ch soil moisture unit fix). +- Generator: `python tests/fixtures/regenerate_golden.py` in the environment + created from `ci/golden-env.lock` plus `ci/golden-pip.lock` (micromamba, + environment name `golden`; linux-64, Python 3.13.7, pcraster 4.4.2, + gdal 3.11.5, numpy 2.3.4), run on Ubuntu 24.04.2 under WSL2 on a developer + workstation (glibc 2.39-0ubuntu8.4, x86_64, Intel Core i7-12700H). +- Why this host is acceptable as the generator: byte identity is a property of + the C library and CPU, not of the machine, so a host is a valid generator + only if it reproduces the runner's bytes. That was verified before promoting + anything: the run produced the previous goldens' exact digests for the 37 + files this change does not touch, and the exact digests the `exact` job on + `ubuntu-24.04` reported for the 8 files it does, so the promoted bytes are + the runner's bytes. `RUBEM_EXACT_GOLDEN=1 pytest tests -m exact` passes in + this environment. +- The `ubuntu-24.04` runner image remains the canonical reference: if the + `exact` job ever disagrees with these bytes, the runner is right and the + goldens must be regenerated there, recording the new provenance here. + +### Impact of the correction (previous goldens vs regenerated) + +Maximum absolute difference over valid (non-nodata) cells/values, both runs +executed on the same host so the numbers isolate the formula change from +cross-environment float noise: + +| Variable | Rasters | Time series CSV | +|---|---|---| +| itp | 0.000000e+00 | 0.000000e+00 | +| bfw | 0.000000e+00 | 0.000000e+00 | +| srn | 1.026310e+02 | 3.522971e+01 | +| eta | 0.000000e+00 | 0.000000e+00 | +| lfw | 0.000000e+00 | 0.000000e+00 | +| rec | 0.000000e+00 | 0.000000e+00 | +| smc | 1.026310e+02 | 3.523000e+01 | +| rnf | 1.026310e+02 | 3.522970e+01 | +| arn | 2.423833e+01 | 1.820190e+01 | + +### Checksums replaced by this change + +``` +srn00000.002 83165b7a643e73f62f7724681863f591a31c3b45c19b875601d621a9b56bb2e6 -> 35fc364b2e309f007aedf027433e1947bb28ce99014ffc4d4cad073022b3f0ac +smc00000.002 88999e794f1625cb03e418a7335e8eea853c7bc84f2f57fef36d001204aeb3ba -> 877cab02bbeeb0c8be4190f8ae3a4e2fde65793fb8ef2cb9cf15cd95e807fb36 +rnf00000.002 058580a89fb1137e61abcd9c7aa74399963cc03c208a80605c1b752b03749694 -> d75b07fc85708ccf5a2da774946dc5e2b534b59caf228c1838ddfb660e5870a3 +arn00000.002 59d42872d6adc7cf5a0a7da4e8daacfeb8f3c5be53c79bc8981af141d46aaaa2 -> 4cd64f93219c0bec9d916c776d09437ee514ede9e4aaf4a6007a1657291151ee +srn0000002.tif a2f624db3e41808be94e02f549fe0363db7faf65fc06d6fe461073a4d26e288a -> cf608b6e227a2ee934b0988eb616ce67ef6c3c549acd936b0bafdf4a2d6d3de8 +smc0000002.tif 46d96253aa9047be531a02597cd350c65ac97db438f0194571842d38563a903d -> 9d8ead92ec46bf7dd074eda97e2acbf1371c055aa803884b84f1bfbf96f4546d +rnf0000002.tif 21a583574ea1af17882fb35c77ee37cb18b112b7929b6daa5ec2ee3ff22ca80f -> e5bf7e18e4dac6c726947f0bf7f60f3cd70927aec882eeb43c566b2c5d40fec9 +arn0000002.tif 8fc702536057758ce89d0e76a6328b42e0744378cde29999901a6a75493a2c79 -> 5c36bb78a75dbbe93e6b50f7ae1ac3964362caedb7ee8e8786501e50c751a10b +tss_srn.csv fa87eaf67f6b23cec33776c3c76d4164d50663542e531c117a8abb6f1abf441e -> 66da9bb684f1873d1739588853c1f8109b35f295354022c32f8975499357e5fe +tss_smc.csv ea3c1d32cb8892d74a1d375c352fead84e83e4d591e8ad345cede2f4e68928b8 -> 95c54fc37ae5f5ff7e277aabe3d2fe0ebb2101aae620971c0e86065b87a94e9c +tss_rnf.csv 70e7bb1ce8dba75795cede407eff3dd445721812165ff85f554bf3bca42bf714 -> 2882d376e33facd35295ccf2d059e843316252a69489080382bab33f21e6c1ba +tss_arn.csv b44aa9fcb00b450e1dda4c00b082335de2c775da951ad5b13402f1e8326031c3 -> a51bdb9c3ca82657f10ea5bb545bf8036fd35716b5c9db6bcd751b1485428531 +``` + +## The k_sat lookup table correction The regression configuration in `tests/integration/test_cli.py` mapped `TABLES.k_sat` to `txt/soil/Tsat.txt`, the same file already used for @@ -8,7 +90,7 @@ The regression configuration in `tests/integration/test_cli.py` mapped fixture `txt/soil/Kr.txt`. The mapping was corrected and the goldens were regenerated once with `tests/fixtures/regenerate_golden.py`. -## Generating environment and recipe +### Generating environment and recipe - Base commit: `fb740f51aeaa38c45f93dd011078f954e2804bfd` (`main`) - Generator: the `exact` CI job on a GitHub-hosted runner of the `ubuntu-24.04` @@ -58,7 +140,7 @@ regenerated once with `tests/fixtures/regenerate_golden.py`. model; a scoped `.gitattributes` rule (`tests/fixtures/base/out/** -text`) keeps git from normalizing any golden byte. -## Impact of the correction (legacy vs regenerated) +### Impact of the correction (legacy vs regenerated) Maximum absolute difference over valid (non-nodata) cells/values: @@ -79,7 +161,7 @@ the corrected `Kr.txt` values differ from `Tsat.txt`; interception (`itp`) does not depend on `k_sat` and only shows Float32-level noise from the change of generating environment relative to the legacy goldens. -## Legacy golden checksums (before this regeneration) +### Legacy golden checksums (before this regeneration) ``` 9b03293d225604ce34f535b4d3174a2f4f91905669c12396ca0cf4f76d5fa4cd arn00000.001 diff --git a/tests/fixtures/base/out/SHA256SUMS b/tests/fixtures/base/out/SHA256SUMS index fbf2a166..f4e10915 100644 --- a/tests/fixtures/base/out/SHA256SUMS +++ b/tests/fixtures/base/out/SHA256SUMS @@ -1,7 +1,7 @@ ce57ba29d78d4ee34478846feddd5174989cbb98424597c881ecf1d15590ea71 arn00000.001 -59d42872d6adc7cf5a0a7da4e8daacfeb8f3c5be53c79bc8981af141d46aaaa2 arn00000.002 +4cd64f93219c0bec9d916c776d09437ee514ede9e4aaf4a6007a1657291151ee arn00000.002 7514c4f592ede4d784a0c5740b4eb70f5bb125e2ba3ac5f6f23d001abdaacc56 arn0000001.tif -8fc702536057758ce89d0e76a6328b42e0744378cde29999901a6a75493a2c79 arn0000002.tif +5c36bb78a75dbbe93e6b50f7ae1ac3964362caedb7ee8e8786501e50c751a10b arn0000002.tif b6eef24f590c120322826c8278d9c42f2a2de584acdacd28d6677883ca6019f6 bfw00000.001 4f97716a4e12d5e1c638a5a92f1916ecbf86a673ed8483fad97662951b0aa8c0 bfw00000.002 4610b4115df7cea1ddee7eb19cda5e13194c974aabf7465533ccdd643673f4b4 bfw0000001.tif @@ -23,23 +23,23 @@ f08a80ed1ff955178efb36e9ca1e7c9f3aa928122a0b6bf71f312d5695d6ca66 rec00000.001 00310600f10cead3fb093e42e96001de9ecb18f735113f9e14b9d377632b1ada rec0000001.tif 6368a08f67f1149c7769fa7370521fad92475503203faec27e6ce135186c8ff2 rec0000002.tif 33964839e235df79aeffb88b739d038a623018d40b78ba828b67c76737d9a7b0 rnf00000.001 -058580a89fb1137e61abcd9c7aa74399963cc03c208a80605c1b752b03749694 rnf00000.002 +d75b07fc85708ccf5a2da774946dc5e2b534b59caf228c1838ddfb660e5870a3 rnf00000.002 0277d8ab6cd0f63c64b6891d7dc24a5259778736b63e14aba28c4d023834ac8d rnf0000001.tif -21a583574ea1af17882fb35c77ee37cb18b112b7929b6daa5ec2ee3ff22ca80f rnf0000002.tif +e5bf7e18e4dac6c726947f0bf7f60f3cd70927aec882eeb43c566b2c5d40fec9 rnf0000002.tif ebea49d1c10a5f765077200189fc1e522db1ff18afd151ab25da78e4581276c2 smc00000.001 -88999e794f1625cb03e418a7335e8eea853c7bc84f2f57fef36d001204aeb3ba smc00000.002 +877cab02bbeeb0c8be4190f8ae3a4e2fde65793fb8ef2cb9cf15cd95e807fb36 smc00000.002 65ca77bc7f41d3d9921384948a5c8ed412f934b08f4026e2b95bc9831430760c smc0000001.tif -46d96253aa9047be531a02597cd350c65ac97db438f0194571842d38563a903d smc0000002.tif +9d8ead92ec46bf7dd074eda97e2acbf1371c055aa803884b84f1bfbf96f4546d smc0000002.tif fcd21ad7fa2da8ff4a1d0256eecc579c9cab0fab2c974a15abcbdc37a5f31c5c srn00000.001 -83165b7a643e73f62f7724681863f591a31c3b45c19b875601d621a9b56bb2e6 srn00000.002 +35fc364b2e309f007aedf027433e1947bb28ce99014ffc4d4cad073022b3f0ac srn00000.002 12574eb21b4f3a0fe2f8a9337fc8dae824f196bf96e4c467afe8e9cc98c6bff3 srn0000001.tif -a2f624db3e41808be94e02f549fe0363db7faf65fc06d6fe461073a4d26e288a srn0000002.tif -b44aa9fcb00b450e1dda4c00b082335de2c775da951ad5b13402f1e8326031c3 tss_arn.csv +cf608b6e227a2ee934b0988eb616ce67ef6c3c549acd936b0bafdf4a2d6d3de8 srn0000002.tif +a51bdb9c3ca82657f10ea5bb545bf8036fd35716b5c9db6bcd751b1485428531 tss_arn.csv 128f1d4a8aaaf79e485e71aa8efdbe0df62e802a5c5adcc3e9c4d8cad33fce3e tss_bfw.csv e39b45e57968ae5d8683d2093068bb3be953b8d2d6b7ac5f0e82a9aa63b73115 tss_eta.csv 4ec1520f89c13facc028e716b3de625eacaf28da7f9c2e7865c8eb519e082b99 tss_itp.csv 8c7ffcea39c7590980edb241f7424d064ffdee0ca6a5f15e6dffb8e423b8fb97 tss_lfw.csv 8c7ffcea39c7590980edb241f7424d064ffdee0ca6a5f15e6dffb8e423b8fb97 tss_rec.csv -70e7bb1ce8dba75795cede407eff3dd445721812165ff85f554bf3bca42bf714 tss_rnf.csv -ea3c1d32cb8892d74a1d375c352fead84e83e4d591e8ad345cede2f4e68928b8 tss_smc.csv -fa87eaf67f6b23cec33776c3c76d4164d50663542e531c117a8abb6f1abf441e tss_srn.csv +2882d376e33facd35295ccf2d059e843316252a69489080382bab33f21e6c1ba tss_rnf.csv +95c54fc37ae5f5ff7e277aabe3d2fe0ebb2101aae620971c0e86065b87a94e9c tss_smc.csv +66da9bb684f1873d1739588853c1f8109b35f295354022c32f8975499357e5fe tss_srn.csv diff --git a/tests/fixtures/base/out/arn00000.002 b/tests/fixtures/base/out/arn00000.002 index 8389a0ea..2d971c7f 100644 Binary files a/tests/fixtures/base/out/arn00000.002 and b/tests/fixtures/base/out/arn00000.002 differ diff --git a/tests/fixtures/base/out/arn0000002.tif b/tests/fixtures/base/out/arn0000002.tif index 6c293c5b..d3b7563b 100644 Binary files a/tests/fixtures/base/out/arn0000002.tif and b/tests/fixtures/base/out/arn0000002.tif differ diff --git a/tests/fixtures/base/out/rnf00000.002 b/tests/fixtures/base/out/rnf00000.002 index 52d35ead..912147ff 100644 Binary files a/tests/fixtures/base/out/rnf00000.002 and b/tests/fixtures/base/out/rnf00000.002 differ diff --git a/tests/fixtures/base/out/rnf0000002.tif b/tests/fixtures/base/out/rnf0000002.tif index c93bbacd..b6e65a97 100644 Binary files a/tests/fixtures/base/out/rnf0000002.tif and b/tests/fixtures/base/out/rnf0000002.tif differ diff --git a/tests/fixtures/base/out/smc00000.002 b/tests/fixtures/base/out/smc00000.002 index 47d17f2f..ed97c77d 100644 Binary files a/tests/fixtures/base/out/smc00000.002 and b/tests/fixtures/base/out/smc00000.002 differ diff --git a/tests/fixtures/base/out/smc0000002.tif b/tests/fixtures/base/out/smc0000002.tif index 39c93ae7..46fc8730 100644 Binary files a/tests/fixtures/base/out/smc0000002.tif and b/tests/fixtures/base/out/smc0000002.tif differ diff --git a/tests/fixtures/base/out/srn00000.002 b/tests/fixtures/base/out/srn00000.002 index 36dfbc52..48ba0fab 100644 Binary files a/tests/fixtures/base/out/srn00000.002 and b/tests/fixtures/base/out/srn00000.002 differ diff --git a/tests/fixtures/base/out/srn0000002.tif b/tests/fixtures/base/out/srn0000002.tif index f674ce4f..adad1db9 100644 Binary files a/tests/fixtures/base/out/srn0000002.tif and b/tests/fixtures/base/out/srn0000002.tif differ diff --git a/tests/fixtures/base/out/tss_arn.csv b/tests/fixtures/base/out/tss_arn.csv index 4e80c1a5..0752d4df 100644 --- a/tests/fixtures/base/out/tss_arn.csv +++ b/tests/fixtures/base/out/tss_arn.csv @@ -1,3 +1,3 @@ 0;1;2;3;4;5 1;10.5398;3.26794;44.8766;38.358;5.81181 -2;8.9387;3.00628;38.6051;32.4698;4.59128 +2;12.2859;6.2564;56.807;46.2034;6.36679 diff --git a/tests/fixtures/base/out/tss_rnf.csv b/tests/fixtures/base/out/tss_rnf.csv index 75dc2abc..098f0ce9 100644 --- a/tests/fixtures/base/out/tss_rnf.csv +++ b/tests/fixtures/base/out/tss_rnf.csv @@ -1,3 +1,3 @@ 0;1;2;3;4;5 1;107.672;47.7744;50.817;25.1352;15.6187 -2;11.0839;33.8603;14.8141;9.87091;57.6293 +2;38.5355;69.09;49.5244;23.0809;57.6293 diff --git a/tests/fixtures/base/out/tss_smc.csv b/tests/fixtures/base/out/tss_smc.csv index e5c3d398..04828da5 100644 --- a/tests/fixtures/base/out/tss_smc.csv +++ b/tests/fixtures/base/out/tss_smc.csv @@ -1,3 +1,3 @@ 0;1;2;3;4;5 1;222.377;1155.45;629.084;490.444;323.4 -2;257.549;1097.1;644.794;424.996;323.4 +2;230.097;1061.87;610.084;411.786;323.4 diff --git a/tests/fixtures/base/out/tss_srn.csv b/tests/fixtures/base/out/tss_srn.csv index e187f8bd..936efbe7 100644 --- a/tests/fixtures/base/out/tss_srn.csv +++ b/tests/fixtures/base/out/tss_srn.csv @@ -1,3 +1,3 @@ 0;1;2;3;4;5 1;92.0536;6.3904;35.1982;9.51649;0 -2;1.02627;0.717492;0.847523;0.32255;39.3699 +2;28.4779;35.9472;35.5578;13.5326;39.3699 diff --git a/tests/unit/hydrological_processes/test_module_surface_runoff.py b/tests/unit/hydrological_processes/test_module_surface_runoff.py index 07dbdc02..c15693af 100644 --- a/tests/unit/hydrological_processes/test_module_surface_runoff.py +++ b/tests/unit/hydrological_processes/test_module_surface_runoff.py @@ -12,43 +12,49 @@ def setup(self): @pytest.mark.unit def test_chCalc(self): - tur = pcr.scalar(1.0) + tur = pcr.scalar(50.0) dg = pcr.scalar(1.0) zr = pcr.scalar(1.0) - tsat = pcr.scalar(1.0) - b = pcr.scalar(1.0) + tsat = pcr.scalar(200.0) + b = pcr.scalar(2.0) field = SurfaceRunoff.get_coef_soil_moist_conditions(tur, dg, zr, tsat, b) result = generalfunctions.getCellValue(field, 0, 0) - expected = 0.1 + expected = 0.0625 assert result == pytest.approx(expected) @pytest.mark.unit def test_chCalc_dg_eq_0(self): - tur = pcr.scalar(1.0) + """Ch no longer converts the soil moisture content, so Dg cannot divide by zero.""" + tur = pcr.scalar(50.0) dg = pcr.scalar(0.0) zr = pcr.scalar(1.0) - tsat = pcr.scalar(1.0) - b = pcr.scalar(1.0) - with pytest.raises(RuntimeError, match="pcrfdiv: operator /: Domain Error"): - SurfaceRunoff.get_coef_soil_moist_conditions(tur, dg, zr, tsat, b) + tsat = pcr.scalar(200.0) + b = pcr.scalar(2.0) + field = SurfaceRunoff.get_coef_soil_moist_conditions(tur, dg, zr, tsat, b) + result = generalfunctions.getCellValue(field, 0, 0) + expected = 0.0625 + assert result == pytest.approx(expected) @pytest.mark.unit def test_chCalc_Zr_eq_0(self): - tur = pcr.scalar(1.0) + """Ch no longer converts the soil moisture content, so Zr cannot divide by zero.""" + tur = pcr.scalar(50.0) dg = pcr.scalar(1.0) zr = pcr.scalar(0.0) - tsat = pcr.scalar(1.0) - b = pcr.scalar(1.0) - with pytest.raises(RuntimeError, match="pcrfdiv: operator /: Domain Error"): - SurfaceRunoff.get_coef_soil_moist_conditions(tur, dg, zr, tsat, b) + tsat = pcr.scalar(200.0) + b = pcr.scalar(2.0) + field = SurfaceRunoff.get_coef_soil_moist_conditions(tur, dg, zr, tsat, b) + result = generalfunctions.getCellValue(field, 0, 0) + expected = 0.0625 + assert result == pytest.approx(expected) @pytest.mark.unit def test_chCalc_Tsat_eq_0(self): - tur = pcr.scalar(1.0) + tur = pcr.scalar(50.0) dg = pcr.scalar(1.0) zr = pcr.scalar(1.0) tsat = pcr.scalar(0.0) - b = pcr.scalar(1.0) + b = pcr.scalar(2.0) with pytest.raises(RuntimeError, match="pcrfdiv: operator /: Domain Error"): SurfaceRunoff.get_coef_soil_moist_conditions(tur, dg, zr, tsat, b)