<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 8 April 2016 at 09:18, Rick Walsh <span dir="ltr"><<a href="mailto:rickmwalsh@gmail.com" target="_blank">rickmwalsh@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Robert,<br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On 7 April 2016 at 23:18, Robert Helling <span dir="ltr"><<a href="mailto:helling@atdotde.de" target="_blank">helling@atdotde.de</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5"><div style="word-wrap:break-word">Rick,<div><span><br><div><blockquote type="cite"><div>On 07.04.2016, at 10:37, Rick Walsh <<a href="mailto:rickmwalsh@gmail.com" target="_blank">rickmwalsh@gmail.com</a>> wrote:</div><br><div><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">The variable deco_time is used for the CVA iterations to calculate the VPM-B</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">ceiling. Only positive values should be allowed, otherwise we can get some odd</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">results in a few cases, such as ceilings being calculated when they shouldn't.</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"></div></blockquote></div><div><br></div></span>I think your analysis was only partially correct. Indeed the problem occurred when the deco time got negative. </div><div><br></div><div>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. </div><div><br></div><div>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. </div><div><br></div><div>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).</div><div><br></div><div></div></div><br></div></div><div style="word-wrap:break-word"><div></div><div><br></div><div>Could you please look at this patch and confirm or dispute my solution?</div><div><br></div><div><br></div></div></blockquote><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div></div></div></div></blockquote><div>An alternative conditional that would work both ways could be:</div><div>     if  ((entry->ceiling > 0) && (entry->ceiling >= first_ceiling) { </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>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):</div><div>- on the initial iteration runs, with the initial gradients (most conservative - ignoring the CVA benefit), a shallow ceiling was calculated</div><div>- on the CVA iteration, no ceiling was calculated because the allowable gradients are a little more aggressive, but <span style="font-size:12.8px">time_clear_ceiling</span><span style="font-size:12.8px"> 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)</span></div><div><span style="font-size:12.8px">- on the next CVA iteration, the updated gradients were wrong because of the negative deco_time</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">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).</span></div><div><br></div><div>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.</div><div><br></div><div>Cheers,</div><div><br></div><div>Rick</div></div></div></div>
</blockquote></div><br></div></div>