core: Calculate EndGF for all dives

[michaelandreen]

Signed-off-by: Michael Andreen michael@andreen.dev

Describe the pull request:

• Bug fix
• Functional change
• New feature
• Code cleanup
• Build system change
• Documentation change
• Language translation

Pull request long description:

Changes made:

Related issues:

Additional information:

Release note:

Documentation change:

Mentions:

[michaelandreen]

If the divetrip check is done first then we end up going through the whole dive list if all the dives in the trip are within 48 hours.

[bstoeger]

Paging @atdotde, since I have no idea what this is about.

[atdotde]

Hmm. Since the tissue calculations are quite expensive computationally, I would have done this a bit differently (and not every time we compute a dive list): Add additional fields final_surface_gf and max_gf (for good measure) to struct dive and initialize those to 0. As the surface GFs are calculated anyway in profile.c when a dive is displayed, I would update the struct dive accordingly, when it is processed by the code in profile.c. Note that this should mark the dive as modified and the value should be saved and loaded accordingly (both to git and xml). And of course it should be displayed in the dive list.

I don't really understand what is the connection to dive trips (as these values are per dive not per trip). If you change anything here, please make it a separate commit.

Bonus points if you make these values available to the statistics module has well (waving to @bstoeger).

[bstoeger]

Bonus points if you make these values available to the statistics module has well (waving to @bstoeger).

Sure, when the rest is settled, I can do that.

[michaelandreen]

Hmm. Since the tissue calculations are quite expensive computationally, I would have done this a bit differently (and not every time we compute a dive list): Add additional fields final_surface_gf and max_gf (for good measure) to struct dive and initialize those to 0. As the surface GFs are calculated anyway in profile.c when a dive is displayed, I would update the struct dive accordingly, when it is processed by the code in profile.c. Note that this should mark the dive as modified and the value should be saved and loaded accordingly (both to git and xml). And of course it should be displayed in the dive list.

I'm not so sure about setting these values when they are processed in profile.c. Currently most of the call chain has the dive has const and that is pretty clean. It could be modified so the values are passed up the call chain to ProfileWidget though return values or out parameters. However, I think the user interface of updating these when clicking is a bit weird. I guess we would have to put new commands on the undo stack and that would happen from what seems like a natural read-only operation.

There is also the problem of saving bad values. A computer might not have enough space for all dives at the set sampling rate, so a dive might be watched values saved before the an earlier dive is added manually. It might also have the wrong time set. We would have to remember to reset these values so they are recalculated if the current dive, or previous dives, changes in ways that would affect them. Then we have the risk of bugs causing the wrong values to be saved.

So, I think it is much cleaner to calculate these values on startup, assuming that it doesn't take too long. Then a restart will always get fresh values with the best available information. With my naive proof of concept it's hard to tell on for the ~500 dives I have in subsurface. Using a debug build it might take a second or two longer, but it's dwarfed by the time it takes to check the cloud servers. I would have to do some more thorough profiling to be sure. There is the possibility of saving the the deco_state at the end of each dive, that way we would only have to check the previous dive instead of going back until we have a 48 hour gap. Maybe having thousands of dives is still a problem, but then we might opt for having it configurable to do the calculations or skip them if it takes too long. Would also have to check how adding calculations for additional parameters affects things.

I don't really understand what is the connection to dive trips (as these values are per dive not per trip). If you change anything here, please make it a separate commit.

It was just something I found when enabling DECO_CALC_DEBUG. For a recent dive trip it continued to go through all dives since that continue hit instead of the break for the 48 hour limit. I added it to the draft just so I didn't forget about it and could raise the question.

It seems to be an optimization, but ends up skipping the real terminal case for the loop and checking more dives than it has to (though they are still ignored in the actual calculation later). I also think the logic behind it seems a bit bogus. I have a case where I spent one week at one dive resort, then traveled by car and boat to another resort of a 2nd week of diving. I put these two resorts as different dive trips and with this logic about skipping dives from other trips the first dive at the 2nd resort ends up with slightly lower end GF. It's not obvious in the user interface that organizing trips in this way would affect deco calculations.

[dirkhh]

Conversation here has stopped. Obviously I should have followed up much earlier.
What I don't quite understand is how this gets used - it adds a new field to the model and then... what?
I must be missing something. 😇

[michaelandreen]

Conversation here has stopped. Obviously I should have followed up much earlier.

Christmas, New Year and work got in the way, so not much has happened =)

What I don't quite understand is how this gets used - it adds a new field to the model and then... what?
I must be missing something.

I would say that the most useful thing for most people is to look at that number and compare it to how tired they are after the dive. If you're more tired after dives when it's over a certain value then you can use that has a hint when setting the conservatism on your dive computer.

For me it's mostly that it's more data and data is fun =)

[atdotde]

I still think if we want to present these values, we should store them in file and not compute all the deco information for all the dives when we load a log file or make any change. A log file can contain thousands of dives.

Yes, when this feature is new, this will modify all the dives in the log (to add this value) and it might be a bit tricky to keep track of which dives were modified and thus need saving (probably everything within 48h of the dive originally modified) but I think this is worth it.