lukeshope / hipims-ocl

The high-performance integrated modelling system, for hydraulic and hydrological simulations
GNU General Public License v3.0
21 stars 10 forks source link

Hard time getting correct(ish) results. #14

Open ninnghazad opened 4 years ago

ninnghazad commented 4 years ago

I'm having a hard time validating results against other simulations or even some calculator math. Uploaded my example config and data dir here: test.7z It's a small village and a lot of rain. I always end up with way too much water, about 2-5 times as much as expected. i am using the godunov scheme, the other one wouldn't work at all using rain from csv files. (it would only use the water i would give it using a depth dataSource). The logs seem ok, it determines correct resolution from dem (1m). Tried single and double precision, and gave the same manning values as for the other simulations, used the example as a basis for the config. Using double precision, the difference isn't as pronounced after like 10mins of simulation, but at the end of the full hour it too is way off. Can anybody give me a hint at what it is i am doing wrong here?

ninnghazad commented 4 years ago

Reducing the courant limit from 0.5 to 0.25 drastically reduced the difference:

max_depth hipims-ocl: STATISTICS_MAXIMUM=46.703359982255 STATISTICS_MEAN=0.051732039615833 STATISTICS_MINIMUM=8.1835056903401e-05 STATISTICS_STDDEV=0.22778051706715

max_depth anuga: STATISTICS_MAXIMUM=13.571899414063 STATISTICS_MEAN=0.059289672847105 STATISTICS_MINIMUM=0.0009765625 STATISTICS_STDDEV=0.24373524740894

the unrealistic maxima are due to a blip in the dem. sadly but expected, halfing the courant limit has roughly doubled simulation time. (to about 2 hours) i will try another run with an even lower courant limit to see if can match anuga's results with a difference below 5%.

ninnghazad commented 4 years ago

Further tightening the courant limit further closed the gap, and further increased runtime. I wonder if this is just a limitation of the applied scheme. I will compare another simulation using a different DEM, the slopes are too extreme in the current one for the scheme to work nicely.

lukeshope commented 4 years ago

Hello,

Sorry for the time it's taken to get back to you on this. I did look at your problems at the weekend. There are two main factors at play here:

In my tests at the weekend, I was able to confirm the volume errors remained under 1% provided the CFL number wasn't too large.

Do you want to try running the v2 debug executable? The MUSCL-Hancock method should work properly and further reduce the volume error, provided you have enough memory to run it.

I'd suggest always using double precision. HiPIMS uses the free-surface level as one of its core variables, which in single precision can introduce significant errors with very shallow depths.

Finally, it would be great to hear more about your project, if you're able to share it. Feel free to email me if I can help.

ninnghazad commented 4 years ago

Thank you for taking the time. I checked out the new release, and indeed the MUSCL-Hancock method now works, as far as i could tell. First results look good, but i will do more comparisons in the coming days/weeks. I am still evaluating, but i am hoping to be able to speed up simulations to come using this over ANUGA (and some proprietaries not worth mentioning). It would be used to create flood hazard maps for extreme rains for cities in germany. We need a lot of these and current simulation times are prohibitive.

For now i will continue comparing results of HiPIMS vs different kernels of ANUGA and other results i have laying around.

You mentioned adding/subtracting quantities at a different frequency - is this a fixed frequency or in relation to the "main" dt or maybe the amount of change towards the quantity?

Oh, and i had to make a small change to get it to compile (g++9.2):

diff -r tmp/hipims-ocl-0.2/src/Domain/Cartesian/CDomainCartesian.cpp src/Domain/Cartesian/CDomainCartesian.cpp
751c751
<                       dVolume += ( max(0.0, this->dCellStates[i].s[0] - this->dBedElevations[i]) ) *
---
>                       dVolume += ( std::max(0.0, this->dCellStates[i].s[0] - this->dBedElevations[i]) ) *
754c754
<                       dVolume += ( max(0.0, this->fCellStates[i].s[0] - this->fBedElevations[i]) ) *
---
>                       dVolume += ( std::max(0.0f, this->fCellStates[i].s[0] - this->fBedElevations[i]) ) *

diff -r tmp/hipims-ocl-0.2/src/Domain/CDomain.cpp src/Domain/CDomain.cpp
339c339
<                       Util::round( ( this->getBedElevation( ulCellID ) + max(-1e-12, dValue) ), ucRounding ) 
---
>                       Util::round( ( this->getBedElevation( ulCellID ) + std::max(-1e-12, dValue) ), ucRounding ) 
344c344
<                       Util::round( ( this->getBedElevation( ulCellID ) + max(-1e-12, dValue) ), ucRounding )
---
>                       Util::round( ( this->getBedElevation( ulCellID ) + std::max(-1e-12, dValue) ), ucRounding )
lukeshope commented 4 years ago

Sounds good, just let me know if I can help, but glad you're getting sensible results.

I haven't used ANUGA (or its GPU version) before, mainly as it didn't exist when I started writing HiPIMS. I can't pretend that HiPIMS it's anything other than research software, which though it has been fairly widely used in various publications over many years, may never offer something as polished or refined as ANUGA with the significant resources behind it.

If you need to run simulations at the regional scale versus individual towns and cities, depending on the level of computing resource you have access to, then the main benefit of HiPIMS would be its domain decomposition. You should be able to scale to multiple nodes with multiple GPUs, using MPI and its domain links.

The hydrological clock used for adding rain to the domain is currently hardcoded in one of the OpenCL header files, the first definition here. It wouldn't be much work to make this configurable, I can certainly do that.

Thanks for the heads up about the compiler. Hasn't been compiled using the later version of g++ at my end for a while. Will patch.