Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decrease relative tolerance from 1e-5 to 1e-7 #1874

Merged
merged 8 commits into from
Oct 8, 2024
Merged

Decrease relative tolerance from 1e-5 to 1e-7 #1874

merged 8 commits into from
Oct 8, 2024

Conversation

visr
Copy link
Member

@visr visr commented Oct 7, 2024

With #1819 the state goes further away from 0 with time. That means we rely more on the relative tolerance than the absolute tolerance, see the SciML docs on the topic. Hence I found that reducing the abstol had no effect, but reducing the default reltol by two orders of magnitude is enough to get only negligible leaky Terminals with occasional very brief leaks of 1e-8 m3/s. It is also enough to minimize the infiltration error to acceptable levels as can be seen in #1863 (comment).

For HWS this slows down simulation from 9.1s to 12.3s, which isn't too bad for a two orders of magnitude reduction in relative tolerance. Rechecking HWS performance gives a speedup! 12.4s for 1e-8, 8.1s for 1e-7, 10.2s for 1e-6. So 1e-7 seems optimal. Not sure what went wrong last time, but this seems pretty consistent. This is with both absolute and relative set to the same number.

Fixes #1851
Fixes #1863

With #1819 the state goes further away from 0 with time. That meanse we rely more on the relative tolerance than the aboslute tolerance.

Reducing the default `reltol` by two orders of magnitude is enough to get only negligible leaky Terminals with occasional very brief leaks of 1e-8 m3/s. It is also enough to minimize the infiltration error to acceptable levels as can be seen in #1863 (comment).

For HWS this slows down simulation from 9.1s to 12.3s, which isn't too bad for a two orders of magnitude reduction in relative tolerance.

https://docs.sciml.ai/DiffEqDocs/stable/basics/faq/#What-does-tolerance-mean-and-how-much-error-should-I-expect
@visr
Copy link
Member Author

visr commented Oct 7, 2024

Perhaps for simplicity/consistency we should set the abstol to 1e-7 as well.

@Huite
Copy link
Contributor

Huite commented Oct 7, 2024

Hence I found that reducing the abstol had no effect, but reducing the default reltol by two orders of magnitude is enough to get only negligible leaky Terminals with occasional very brief leaks of 1e-8 m3/s.

This seems very obvious in retrospect, indeed.
It's a bit unintuitive coming from a regular head/storage formulation, since those are clearly more bounded.
Resetting to 0 is still a good idea for this reason (maybe not every saveat, but maybe every change of boundary condition), since the solver is getting a shock in those cases anyway.

@visr
Copy link
Member Author

visr commented Oct 7, 2024

It's hard to find a good time for resetting though that doesn't cause issues for BMI, which currently assumes there is never any resetting. Also saveat can be infinite. Not resetting also makes it easy to get the total water balance over time.

@Huite
Copy link
Contributor

Huite commented Oct 7, 2024

It's hard to find a good time for resetting though that doesn't cause issues for BMI, which currently assumes there is never any resetting. Also saveat can be infinite. Not resetting also makes it easy to get the total water balance over time.

I understand, but a boundary condition that continually supplies water will see its flow grow and grow; at that point the reltol becomes meaningless.

@visr
Copy link
Member Author

visr commented Oct 7, 2024

In what way meaningless? Do you mean floating point accuracy issues? Perhaps a yearly reset would be predictable enough for BMI to work around, although the BMI driver would have to be calendar aware.

@Huite
Copy link
Contributor

Huite commented Oct 8, 2024

Let's say you have a flow boundary constantly pumping in water and the flow on the edge accumulates monotonically, with 1000 m3/d; let's further assume that flow is (nigh) steady-state.

At t=1 day, flow = 1000; relative tolerance value is reltol * 1000;
At t=1000 days, flow = 100 000 days; relative tolerance value is reltol * 1_000_000.

I.e. the tolerance has effectively become a 1000 times relaxed, right? While the physical meaning of the model remains the same.

This was referenced Oct 8, 2024
@visr visr merged commit 89e0e9f into main Oct 8, 2024
25 of 27 checks passed
@visr visr deleted the reltolerance branch October 8, 2024 12:09
evetion added a commit that referenced this pull request Oct 9, 2024
Fixes #1892 for now to speed up our CI. We should further investigate
why the new tolerances (#1874) have slowed this model down so much. This
also again stresses the need for a good benchmark suite, which should've
caught it.
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.

Error in exchange waterbalance coupler Ribasim-MF-MS [bug] Terminals sometimes "leak" water
2 participants