dankelley / oce

R package for oceanographic processing
http://dankelley.github.io/oce/
GNU General Public License v3.0
145 stars 42 forks source link

CTD automatic conversion of PSI to dbar #1166

Closed dankelley closed 5 years ago

dankelley commented 7 years ago

Clarks opinion sought ... skip to the end if this is too long...

I was cleaning up emails and ran across a .cnv file that had pressure in PSI. When you read such a file, oce says e.g.

Warning messages:
In cnvName2oceName(lines[nameLines[iline]], columns, debug = debug -  :
  this .cnv file contains pressure in PSI, but [["pressure"]] will return in dbar

and this is, indeed, what happens, e.g.

> d[["pressure"]][1]
[1] 0.1509952
> d@data$pressure[1]
[1] 0.219
> d@data$pressure[1] * 0.6894757293178
[1] 0.1509952

HOWEVER I am not any longer convinced that this is the best approach. The reason is that the summary listing gives

pressure [psi]                         -0.278   75.443  142.83  15777 prDE        

which, to my mind, suggests that doing d[["pressure"]] would retrieve in PSI.

Proposal. I think there should be a column named pressurePSI that is exactly as in the file, but that a new column called pressure should be invented. I think that's what we do (or should do...must check on this...) for temperature in a non-modern scale. Thoughts, Clark?

richardsc commented 7 years ago

I agree with the proposal. Otherwise we get into a rats nets of having to check the pressure units anytime we do a seawater calculation (as we currently do with conductivity, only because it is much more common to have a variety of conductivity units ...)

dankelley commented 7 years ago

This has been done in develop commit 82713cf3af841e7fcab535eb0e1c90196d07fb5e

Warning message:

Warning message:
In read.ctd.sbe(file, processingLog = processingLog, ...) :
  created 'pressure' from 'pressurePSI'

Tests:

mlcoleman commented 5 years ago

Hmm, I am reading CTD files that have pressure in psi, and indeed I get this message "created 'pressure' from 'pressurePSI'" and a new column. But I think the converted "pressure" -- which I think should be dbar -- is incorrect. It looks like pressure(dbar) is being calculated as pressurePSI / 0.6894757, rather than pressurePSI * 0.6894757. Am I missing something?

dankelley commented 5 years ago

I think you are correct. Starting at line 1142 of R/ctd.sbe.R I see

           res <- oceSetData(res, name="pressure", value=res@data$pressurePSI/0.6894757293168,
                              unit=list(unit=expression("dbar"), scale=""))
            warning("created 'pressure' from 'pressurePSI'")

which is wrong, so I'll change that.

  1. Could you email me (kelley.dan@gmail.com) a sample file, so I can check on my change?
  2. Do you have the ability to build oce from github? (That requires a C compiler, etc.) I ask this because updating a new version to CRAN is a big deal, and we try to do that only once or twice a year, for large things. If you cannot, or if you're in a rush, then you can always do e.g.
    ctd <- oceSetData(ctd, name="pressure",
    value=ctd[["pressure"]]*0.6894757293168^2,
    unit=list(unit=expression(dbar), scale=""),
    note="fix pressure value")
dankelley commented 5 years ago

I've fixed the code, in "develop" branch, commit 1f0ad7b462d328adc535b6753c0714542d3fa53e (click the link to see the "diff" in the code).

mlcoleman commented 5 years ago

great thank you! I'll grab the updated version. just sent a file too.

dankelley commented 5 years ago

cool. I tried

> library(oceanglider)
Loading required package: knitr
> d <- read.oce("~/Dropbox/psi.cnv")
Warning in read.ctd.sbe(file, processingLog = processingLog, ...) :
  created 'salinity' from 'temperature', 'conductivity' and 'pressure'
Warning message:
In read.ctd.sbe(file, processingLog = processingLog, ...) :
  created 'pressure' from 'pressurePSI'
> range(d[["pressure"]])
[1]  0.08204761 90.66261103

so this looks reasonable, given your domain. (I am here not stating your domain, and I renamed your data file for privacy.)

mlcoleman commented 5 years ago

thanks.

dankelley commented 5 years ago

I'll close this issue, since it seems to be fixed, but please feel free to reopen it, if you still have problems or if there are other things to bring up. (Also, feel free to open issues for other topics.)