New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More accurate CNS calculations #1850

Merged
merged 1 commit into from Nov 17, 2018

Conversation

Projects
None yet
5 participants
@willemferguson
Contributor

willemferguson commented Nov 10, 2018

Update table of maximum oxygen exposure durations, used in CNS calulations.
This table shows the official NOAA maximum O2 exposure limits
(in seconds) for different PO2 values. It also gives
slope values for linear interpolation for intermediate PO2 values
between the tabulated PO2 values in the 1st column.
Top & bottom rows are inserted that are not in the NOAA table:
(1) For PO2 > 1.6 the same slope value as between
1.5 & 1.6 is used. This exptrapolation for PO2 > 1.6 likely
gives an underestimate above 1.6 but is better than the
value for PO2=1.6 (45 min). (2) The NOAA table only
tabulates values for PO2 >= 0.6. Since O2-uptake occurs down to
PO2=0.5, the same slope is used as for 0.7 > PO2 > 0.6.
This gives a conservative estimate for 0.6 > PO2 > 0.5. To
preserve the integer structure of the table, all slopes are
given as slope*10: divide by 10 to get the valid slope.
The columns below are:
po2 (mbar), Maximum Single Exposure (seconds), single_slope,
Maximum 24 hour Exposure (seconds), 24h_slope */

Then update Calculations of the CNS for a single dive -
this only takes the first divecomputer into account.
The previous version of the code did a table lookup and
used the max O2 exposure for the next-higher PO2 category.
This gave a shorter max O2 exposure time and a higher CNS
contribution for a specific dive segment, resulting in a
slightly conservative value of CNS, often some 2 - 3 % too high.
This code does an interpolation for PO2 values inbetween
PO2 entries in the lookup table and therefore results in a more
accurate maximum O2 exposure time for that PO2.
The maximum O2 exposure duration for each segment
is also calculated based on the mean depth of the two
samples (start & end) that define each segment. The CNS
contribution of each segment is found by dividing the
time duration of the segment by its maximum exposure duration.
The contributions of all segments of the dive are summed to
get the total CNS% value. This is a partial implementation
of the proposals in Erik Baker's document "Oxygen Toxicity Calculations" */

Overall, this PR does not radically alter the existing CNS calculation,
it only makes it more accurate and more consistent by doing
interpolation and by using mean segment depth to find PO2.

Signed-off-by: willemferguson willemferguson@zoology.up.ac.za

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:

@atdotde @glance-

@janmulder

This comment has been minimized.

Collaborator

janmulder commented Nov 11, 2018

@dirkhh: the Travis build error on Android is valid. Some recent commits in qt-android-cmake break our build.

@dirkhh

This comment has been minimized.

Collaborator

dirkhh commented Nov 13, 2018

@dirkhh: the Travis build error on Android is valid. Some recent commits in qt-android-cmake break our build.

Great. Something else to investigate. Thanks for tracking this down, @janmulder

@dirkhh dirkhh requested a review from atdotde Nov 13, 2018

@dirkhh

This comment has been minimized.

Collaborator

dirkhh commented Nov 13, 2018

Robert, this is outside my comfort zone. Comments?

@atdotde

Only the j-- comment might actually be problematic (but maybe I don't get it), the rest is just coding style.

static double calculate_cns_dive(const struct dive *dive)
{
int n;
size_t j;
const struct divecomputer *dc = &dive->dc;
double cns = 0.0;
/* Caclulate the CNS for each sample in this dive and sum them */
/* Calculate the CNS for each sample in this dive and sum them */

This comment has been minimized.

@atdotde

atdotde Nov 13, 2018

Collaborator

Seems like whitespace only.

This comment has been minimized.

@willemferguson

willemferguson Nov 14, 2018

Contributor

Dropped open line

int o2 = active_o2(dive, dc, sample->time);
po2 = lrint(o2 * depth_to_atm(sample->depth.mm, dive));
po2 = lrint(o2 * depth_to_atm((int)round((sample->depth.mm + psample->depth.mm) / 2.0), dive));

This comment has been minimized.

@atdotde

atdotde Nov 13, 2018

Collaborator

why do you go to float? Integer should be fine.

This comment has been minimized.

@willemferguson
if (po2 > cns_table[j][0])
break;
j--;

This comment has been minimized.

@atdotde

atdotde Nov 13, 2018

Collaborator

Did you leave out this j-- on purpose? I don't know which direction your slope (to the left or to the right) is actually going to work.

This comment has been minimized.

@willemferguson

willemferguson Nov 14, 2018

Contributor

j-- dropped intentionally

/* Find what table-row we should calculate % for */
for (j = 1; j < sizeof(cns_table) / (sizeof(int) * 3); j++)
/* Find the table-row for calculating the maximum exposure at this PO2 */
for (j = 1; j < sizeof(cns_table) / (sizeof(int) * 5); j++)

This comment has been minimized.

@atdotde

atdotde Nov 13, 2018

Collaborator

These indices could be slightly more readable if you typedef'ed an enum with the column names

typedef enum {PO2, MAX_SINGLE_EXPOSURE, SINGLE_SLOPE, MAX_DAILY_EXPOSURE, MAX_DAILY_SLOPE, LAST} cns_headers;

then this 5 becomes LAST and you can use names when accessing the table elements.

@willemferguson willemferguson force-pushed the willemferguson:CNScalcs branch from fd027f0 to 68610de Nov 14, 2018

@willemferguson

This comment has been minimized.

Contributor

willemferguson commented Nov 14, 2018

The updated code above hopefully addresses the issues raised. Thanks for your attention to this.

More accurate CNS calculations (following comments on github)
Update table of maximum oxygen exposure durations, used in CNS calulations.
This table shows the official NOAA maximum O2 exposure limits
(in seconds) for different PO2 values. It also gives
slope values for linear interpolation for intermediate PO2 values
between the tabulated PO2 values in the 1st column.
Top & bottom rows are inserted that are not in the NOAA table:
(1) For PO2 > 1.6 the same slope value as between
1.5 & 1.6 is used. This exptrapolation for PO2 > 1.6 likely
gives an underestimate above 1.6 but is better than the
value for PO2=1.6 (45 min). (2) The NOAA table only
tabulates values for PO2 >= 0.6. Since O2-uptake occurs down to
PO2=0.5, the same slope is used as for 0.7 > PO2 > 0.6.
This gives a conservative estimate for 0.6 > PO2 > 0.5. To
preserve the integer structure of the table, all slopes are
given as slope*10: divide by 10 to get the valid slope.
The columns below are:
po2 (mbar), Maximum Single Exposure (seconds), single_slope,
Maximum 24 hour Exposure (seconds), 24h_slope */

Then update Calculations of the CNS for a single dive  -
this only takes the first divecomputer into account.
The previous version of the code did a table lookup and
used the max O2 exposure for the next-higher PO2 category.
This gave a shorter max O2 exposure time and a higher CNS
contribution for a specific dive segment, resulting in a
slightly conservative value of CNS, often some 2 - 3 % too high.
This code does an interpolation for PO2 values inbetween
PO2 entries in the lookup table and therefore results in a more
accurate maximum O2 exposure time for that PO2.
The maximum O2 exposure duration for each segment
is also calculated based on the mean depth of the two
samples (start & end) that define each segment. The CNS
contribution of each segment is found by dividing the
time duration of the segment by its maximum exposure duration.
The contributions of all segments of the dive are summed to
get the total CNS% value. This is a partial implementation
of the proposals in Erik Baker's document "Oxygen Toxicity Calculations" */

Overall, this PR does not radically alter the existing CNS calculation,
it only makes it more accurate and more consistent by doing
interpolation and by using mean segment depth to find PO2.

Signed-off-by: willemferguson <willemferguson@zoology.up.ac.za>

@willemferguson willemferguson force-pushed the willemferguson:CNScalcs branch from 68610de to 754b680 Nov 14, 2018

@dirkhh

This comment has been minimized.

Collaborator

dirkhh commented Nov 15, 2018

Just FYI, I'm literally in the middle of doing the 4.8.4 release, so this will be merged after that goes out

@dirkhh dirkhh merged commit 1aef221 into Subsurface-divelog:master Nov 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bstoeger

This comment has been minimized.

Collaborator

bstoeger commented Nov 17, 2018

I would think this and #1851 should have a CHANGELOG.md entry? Also the multiple-divemaster thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment