Hi Jan, > On 30 Jun 2015, at 15:38, Dirk Hohndel <[email protected]> wrote: > > Jan, thanks for sending this. I have asked Robert to look at the code from > the algorithmic side, but let me give you some general feedback in > addition to the comments that I made on github:
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 git rebase —interactive and git add -p are both extremely useful when rewriting history in order to make your patches look like you write perfect code right from the start. Furthermore, my git diff found trailing whitespace (showing up as red on black background in my terminal) in a few places. 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)? 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. 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. 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. I think that’s it for now. Will have to do the midterm report next. Best Robert
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
