Implement success

[visr]

This is a follow-up of #2191. An issue with that is that it logged a warning about crashing simulations also when it succeeded. Also it didn't log the bottlenecks when you interrupt a (hanging) simulation.

Now it does both, this is what is shown when interrupting a simulation, stacktrace is clipped off:

┌ Info: Starting a Ribasim simulation.
│   toml_path = c:\Users\visser_mn\Downloads\lhm-sel\lhm-sel.toml
│   cli.ribasim_version = 2025.2.0
│   starttime = 2020-01-01T00:00:00
│   endtime = 2021-01-01T00:00:00
└ @ Ribasim C:\ProgramData\DevDrives\repo\ribasim\Ribasim\core\src\main.jl:40
┌ Warning: Simulation crashed or interrupted.
└ @ Ribasim C:\ProgramData\DevDrives\repo\ribasim\Ribasim\core\src\main.jl:51
┌ Warning: Convergence bottlenecks in descending order of severity:
│   Outlet #591225 = 48782.889544290454
│   Outlet #591226 = 48782.889544290454
│   ManningResistance #340784 = 0.04819449424793999
│   Pump #340303 = 0.045638255628477994
│   Pump #340194 = 0.04563825461806383
└ @ Ribasim C:\ProgramData\DevDrives\repo\ribasim\Ribasim\core\src\logging.jl:55
ERROR: InterruptException:
Stacktrace:
  [1] bracketstrictlymontonic(v::Vector{Float64}, x::Float64, guess::Int64, o::Base.Order.ForwardOrdering)

This implements Base.success(::Model)::Bool. Before we checked the SciML retcode. I noticed that the retcode of an ongoing simulation that gets interrupted is also successfull. So now we check both the retcode and if it is_finished.

[visr]

It was good that this adds is_finished, which now checks if the t at the end is equal to endtime, because it highlighted an issue that I wasn't aware of. Two models had allocation.timestep = 100_000, which doesn't fit exactly in days. So we had a remainder timestep that we didn't take. This is now fixed:

Ribasim/core/src/model.jl

Lines 350 to 359 in e007c98

	for _ in 1:n_allocation_times
	update_allocation!(integrator)
	SciMLBase.step!(integrator, timestep, true)
	end
	# Any possible remaining step (< allocation.timestep) after the last allocation
	dt = tspan[end] - integrator.t
	if dt > 0
	update_allocation!(integrator)
	SciMLBase.step!(integrator, dt, true)
	end

At first I thought SciMLBase.solve! perhaps didn't mean it would end at the endtime, but it does, it was our own custom allocation timestep code that was wrong.

[SouthEndMusic]

Looks good, added some nits

[SouthEndMusic]

Why? Was there a name conflict?

[visr]

Before this PR we overloaded SciMLBase.solve! and SciMLBase.step!, and now they are just Ribasim.solve! and Ribasim.solve!, so internally solve! is Ribasim.solve and we qualify the SciMLBase methods.

The overloading was not really useful and potentially confusing. And with some bad luck they can lead to more invalidations.

[SouthEndMusic]

Suggested change

	if io !== nothing
	if !isnothing(io)

[visr]

I'm a bit hesitant because I believe the io !== nothing can be easier on the compiler. We use === and !== in many places, more than isnothing. See for instance JuliaLang/julia#35585. It might be better in more recent julia versions though.

[SouthEndMusic]

It looks like allocation_times is only used to determine n_allocation_times, so maybe this can be simply

n_allocation_times = int(floor(tspan[end]/timestep))

[visr]

Done, good call.

[SouthEndMusic]

revert?

[visr]

Why? The value is the same. I just need the variable for an extra test of the time at the end through BMI.