search

dankelley / oce

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

read.adp.rdi(): should we obey the sensor-active flag? #1766

Closed dankelley closed 3 years ago

dankelley commented 3 years ago

Note: this stems from https://github.com/dankelley/oce/issues/1765#issuecomment-748395096 and discussion a few comments above that, too. But I thought it needed its own issue, because of the topic change.

Q for @richardsc: if you've been using the sensor-active flag and trust it, then maybe we ought to use that flag in the code too, to blank out data. I see some options, and wonder which (if any) you like.

  1. Maintain present action, storing the flag but not actually doing anything with it.
  2. Produce a warning for each missing sensor, so that e.g. the user would know that adp[["pressure"]] would produce garbage if that sensor had not been active.
  3. Store a NA matrix for any sensor that is not active.
  4. As 3, but with a 0 matrix (as I think Rich Pawlowicz's matlab code does, I think)
  5. Do not store anything, for any sensor that is not active. (This means that e.g. adp[["pressure"]] would return NULL.)
  6. Other schemes that you think of.

I think it makes sense to decide on this before a CRAN release.

Note that even if we make drastic changes, I can add a new argument that would let the user retain things as they are, just in case we think there is any chance of the bit-flag ever being wrong. I don't want a user being frozen out of accessing their data, in such a case. I suppose that arg could be a logical value called retainUnsampled or something (I don't quite like that name, and would like your opinion on this, too.)

richardsc commented 3 years ago

I am honestly not sure that it's worth the work to bother. In all my years of looking at ADCP data, the file from #1765 is the first I've seen whacky pressure readings in a unit that didn't have a pressure sensor. In all other cases I've seen (i.e. more modern firmware) the values recorded in the pressure field are simply 0.

I think that the case of #1765 is simply a VERY old firmware version (remember, we don't even have a manual that matches the file specifications, hence why we re-tooled read.adp.rdi() to read the times differently for that case).

So I vote for "1. Maintain present action, storing the flag but not actually doing anything with it".

dankelley commented 3 years ago

I agree (vote '1'), and am closing the issue. Thanks.