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

[Rick Walsh]

On 8 April 2016 at 09:18, Rick Walsh <rickmwalsh at gmail.com> wrote:

> 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.
>
An alternative conditional that would work both ways could be:
     if  ((entry->ceiling > 0) && (entry->ceiling >= first_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/e910d889/attachment-0001.html>