lisflood-code icon indicating copy to clipboard operation
lisflood-code copied to clipboard

Feature/mct optim fix

Open corentincarton opened this issue 1 year ago • 3 comments

corentincarton avatar Oct 17 '24 21:10 corentincarton

I checked the dis.tss results of the branch master and the current branch using cold.xml. Results after the commit "split and optimization of the new solve1pixel function " are bit identical. Results after "minor optimisations" are slightly different but I think they are still OK:

29.2018 -> 29.2017 25.7968 -> 25.7967 58.7482 -> 58.7476 51.9246 -> 51.9239 ...

doc78 avatar Oct 18 '24 09:10 doc78

Is 'minor optimisations' strictly necessary? I would very much prefer having bit identical results.

ecCinziaMazzetti avatar Oct 18 '24 15:10 ecCinziaMazzetti

I think we can maybe do something in between that allows bit reproducibility. I think moving around some parts of the code (counts, upstream inflow, etc.) helped but maybe not the removal of closureError and the change in the while loop, which is what impacts may the bit reproducibility.

corentincarton avatar Oct 18 '24 15:10 corentincarton

We run some test for speed and result differences adding the commit "minor optimisation" and we found the following: Speed: there is no speed difference between the two versions (with and without minor optimization). Results differences: compared to the feature\mct_optim, the minor optimization commit introduces differences in the order of 0.000001 (absolute values), while in the version without minor optimization the differences are less then 0.0000001 (even if both version are not bit identical to the feature\mct_optim branch). For testing, we used catchments G0629 (GloFAS3arcmin setup), 3 years of daily simulations. The simulation included only SplitRouting optional module, all other optional modules were deactivated. The comparison used the output netcdf files.

doc78 avatar Nov 08 '24 11:11 doc78

@doc78, thanks for this! We did some checks on our side and we also came to the same conclusion. I think we can move forward with this version. @ecCinziaMazzetti and Nikos are currently running some performance and calibration tests, we'll share with you the results of the investigations.

corentincarton avatar Nov 08 '24 11:11 corentincarton