<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Jan,<div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On 30 Jun 2015, at 15:38, Dirk Hohndel <<a href="mailto:dirk@hohndel.org" class="">dirk@hohndel.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Jan, thanks for sending this. I have asked Robert to look at the code from</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">the algorithmic side, but let me give you some general feedback in</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">addition to the comments that I made on github:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote></div><br class=""></div><div class="">I have pulled your patches and now looked at them all combined (i.e. diff’ed to master) and my overall impression is very good. I second all of Dirk’s suggestions (of course, how could I?) and should maybe point out (as I learned this only very recently) that </div><div class=""><br class=""></div><div class="">git rebase —interactive</div><div class=""><br class=""></div><div class="">and</div><div class=""><br class=""></div><div class="">git add -p</div><div class=""><br class=""></div><div class="">are both extremely useful when rewriting history in order to make your patches look like you write perfect code right from the start.</div><div class=""><br class=""></div><div class="">Furthermore, my git diff found trailing whitespace (showing up as red on black background in my terminal) in a few places.</div><div class=""><br class=""></div><div class="">Now for less formal aspects: I computed a few schedules with your code and they seem to be not totally off (I did not compare them to the output of other programs that claim to compute vpm-b, yet). I guess you did. Could you please post those tests (the dives you did your benchmarking on and what the other programs said)?</div><div class=""><br class=""></div><div class="">Something that applied to the old Buehlmann code as well: We should define the constants in useful units, so we don’t have to convert them before we can use them. In particular I am thinking of all half-times (including the regeneration time) which, currently we state in minutes but use (as all times) in seconds. We could simply add „* 60“ in all the initialisations so the multiplication is done at compile time rather than at runtime.</div><div class=""><br class=""></div><div class="">The plan() function is by now a huge monolith. We should break it into digestible pieces. The problem is of course all the state held in its local variables. Maybe those should be put in a struct and a pointer to that passed around.</div><div class=""><br class=""></div><div class="">There is one thing that I did not expect: I printed the final allowable gradients and the deeper and longer the dives get, the gradients increase. I must say I expected the opposite. But maybe that is my flawed understanding of the model.</div><div class=""><br class=""></div><div class="">I think that’s it for now. Will have to do the midterm report next.</div><div class=""><br class=""></div><div class="">Best</div><div class="">Robert</div></body></html>