[PATCH] Profile VPM-B ceiling: ignore negative deco time

[Rick Walsh]

Robert,

On 7 April 2016 at 23:18, Robert Helling <helling at atdotde.de> wrote:

> Rick,
>
> On 07.04.2016, at 10:37, Rick Walsh <rickmwalsh at gmail.com> wrote:
>
> The variable deco_time is used for the CVA iterations to calculate the
> VPM-B
> ceiling. Only positive values should be allowed, otherwise we can get some
> odd
> results in a few cases, such as ceilings being calculated when they
> shouldn't.
>
>
> I think your analysis was only partially correct. Indeed the problem
> occurred when the deco time got negative.
>
> But looking at the code, this should not happen (it would be negative if
> the deepest ceiling occurred after the ceiling had cleared). So your patch
> does not address the actual problem, only the symptom.
>
> As I understand it, the rounding of ceilings to multiples of 3m is really
> to blame: In the if clause to find the deepest ceiling, there was a >= in
> the ceiling comparison. So “deepest” was also found when the ceiling was
> constant at 0m, which made time_deep_ceiling the end of the dive.
>
> Changing that >= to > does not solve the problem either since that is far
> too early (in the xml provided, the first time there was any non-zero
> ceiling as that was rounded to 3m). The way out is really to take the
> ceiling without rounding to multiples of 3m. This si what this patch does
> (unfortunately, it also fixes whitespace damage).
>
>
>
> Could you please look at this patch and confirm or dispute my solution?
>
>
> I have looked at the patch, but I've not had a chance to compile and run
it.  Can I assume you tested John's xml, and it fixed the problem?  From
what I can see, there should be a small ceiling when the conservatism is
+1, and no ceiling when it's +0.  Don't get confused (I did at first) by
the red ceiling, i.e. DC reported, which now always prints - it should
always print on mobile, but on desktop whether or not it prints should be
determined whether that option has been selected.

I agree that we should not consider a ceiling of 0 m.  Using the actual
calculated ceiling, rather than the ceiling rounded to 3 m, is more
rational.  I can't recall, but we might have decided to use (in the planner
VPM-B) the rounded ceiling because it was more consistent with other
implementations.  I could be wrong on that point but it was something that
I definitely considered last year.  If not rounding the ceiling still shows
essentially the same ceiling for planned dives after they have been saved,
I say we should definitely use the not-rounded ceiling.  Either way, we
should use the same value whether or not the user selects the "show
ceilings rounded the 3m" option.

Using >= was deliberate but perhaps misguided. I wanted to pick up the last
time the ceiling was deepest.  Imagine a dive which starts quite deep
(deepest ceiling occurs at some point), the diver ascends (ceiling becomes
shallower), diver stays at the same depth or goes deeper until the previous
deepest ceiling is reached but not exceeded, before starting the final
ascent.  I wanted to consider the second time the deepest ceiling is
reached as the end of the bottom phase of the dive, and base the CVA from
that.  However, if we don't round the ceiling, such that it is calculated
to the mm, the likelihood of reaching exactly the same ceiling is so
unlikely, and the consequence is minor, that using > rather than >= is
worthwhile to avoid the problem with rounded ceilings and zero ceiling.

I only had a few minutes to look at what as happening with John's dives.
My initial impression was (for the problem dive with +0 conservatism):
- on the initial iteration runs, with the initial gradients (most
conservative - ignoring the CVA benefit), a shallow ceiling was calculated
- on the CVA iteration, no ceiling was calculated because the allowable
gradients are a little more aggressive, but time_clear_ceiling was the
value from previous iteration, which meant that deco_time was negative (I
could be completely wrong - it depends on when time_clear_ceiling is
defined / initialized)
- on the next CVA iteration, the updated gradients were wrong because of
the negative deco_time

I must confess, my debugging was very limited.  I really only got as far as
realizing that deco_time was negative, so tried to make that impossible.
My simple change fixed the problem with John's dives, so it must be right
(jk).

If your patch works, and doesn't make the calculated ceiling look incorrect
for planned dives, and other real dives (especially paying attention to
dives with only very shallow ceilings), I think you have the right
solution.  We could apply both patches, but would want to be reasonably
confident that my patch isn't just masking a problem rather than fixing it.

Cheers,

Rick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20160408/9a560d4e/attachment.html>