dankelley / oce

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

subset(argo) by depth does not adjust lengths of lon and lat #1327

Closed dankelley closed 7 years ago

dankelley commented 7 years ago

For example, from the docs:

> plot(subset(argoGrid(argo), pressure > 500 & pressure < 1000), which=5)
Error in as.ctd(x@data$salinity, x@data$temperature, x@data$pressure,  : 
  lengths of salinity and longitude must match but they are 12488 and 2240, respectively

This bug seems to be just in the most recent commit (i.e. an hour old)

dankelley commented 7 years ago

More info:

> summary(subset(argoGrid(argo), pressure > 500 & pressure < 1000))
Argo Summary
------------

* Source:              `"/Users/kelley/git/oce/create_data/argo/6900388_prof.nc"`
* id:                  "6900388"
* Profiles:            210 delayed; 13 adjusted; 0 realtime
* Time ranges from 2005-10-29 13:57:42 to 2011-11-27 05:11:40 with 223 samples and mean increment 9.998355 day
* Data

                                          Min.   Mean    Max.    Dim.  OriginalName       
    latitude [°N]                         48.743 56.715  64.335  223   LATITUDE           
    longitude [°E]                        -60.52 -37.077 -21.385 223   LONGITUDE          
    pressure [dbar]                       4.4    528.19  1750.4  56x40 PRES               
    pressureAdjusted [dbar]               4.4    521.09  1750.4  56x40 PRES_ADJUSTED      
    pressureAdjustedError [dbar]          2.4    2.4     2.4     56x40 PRES_ADJUSTED_ERROR
    salinity [PSS-78]                     34.315 34.988  35.658  56x40 PSAL               
    salinityAdjusted [PSS-78]             34.315 34.988  35.658  56x40 PSAL_ADJUSTED      
    salinityAdjustedError [PSS-78]        0.01   0.01    0.01    56x40 PSAL_ADJUSTED_ERROR
    temperature [°C, ITS-90]              2.3988 6.1784  13.891  56x40 TEMP               
    temperatureAdjusted [°C, ITS-90]      2.3988 6.1784  13.891  56x40 TEMP_ADJUSTED      
    temperatureAdjustedError [°C, ITS-90] 0.002  0.002   0.002   56x40 TEMP_ADJUSTED_ERROR

but

> summary(argo)
Argo Summary
------------

* Source:              `"/Users/kelley/git/oce/create_data/argo/6900388_prof.nc"`
* id:                  "6900388"
* Profiles:            210 delayed; 13 adjusted; 0 realtime
* Time ranges from 2005-10-29 13:57:42 to 2011-11-27 05:11:40 with 223 samples and mean increment 9.998355 day
* Data

                                          Min.   Mean    Max.    Dim.   OriginalName       
    latitude [°N]                         48.743 56.715  64.335  223    LATITUDE           
    longitude [°E]                        -60.52 -37.077 -21.385 223    LONGITUDE          
    pressure [dbar]                       3.5    521.31  6534.6  56x223 PRES               
    pressureAdjusted [dbar]               3.5    519.53  1778.9  56x223 PRES_ADJUSTED      
    pressureAdjustedError [dbar]          2.4    2.4     2.4     56x223 PRES_ADJUSTED_ERROR
    salinity [PSS-78]                     0      34.911  47.899  56x223 PSAL               
    salinityAdjusted [PSS-78]             32.85  34.936  35.842  56x223 PSAL_ADJUSTED      
    salinityAdjustedError [PSS-78]        0.01   0.01    0.01    56x223 PSAL_ADJUSTED_ERROR
    temperature [°C, ITS-90]              -1.564 6.1216  55.997  56x223 TEMP               
    temperatureAdjusted [°C, ITS-90]      -1.564 6.1016  14.914  56x223 TEMP_ADJUSTED      
    temperatureAdjustedError [°C, ITS-90] 0.002  0.002   0.002   56x223 TEMP_ADJUSTED_ERROR
dankelley commented 7 years ago

Fixed in "develop" branch commit f06e262e4d1fe6b3c01c828d006694165332b1b4

richardsc commented 7 years ago

Perhaps I'm a bit confused, but by my thinking if you're subsetting by depth the length of lat/lon shouldn't change. That's because the length of lat should match the number of profiles, but subsetting by depth should not trim the number of profiles ...

I'm going to re-open this until I can convince myself that the behaviour about is correct.

richardsc commented 7 years ago

Not sure if this is related, but I tried the above subset on another file (the one from #1325) and it doesn't even work:

library(oce)
a <- read.oce('~/Downloads/4901783_prof.nc')
as <- subset(argoGrid(a), pressure > 500 & pressure < 1000)
Error in res@data[[ifield]][, keep] : 
  (subscript) logical subscript too long
> traceback()
3: .local(x, ...)
2: subset(argoGrid(a), pressure > 500 & pressure < 1000)
1: subset(argoGrid(a), pressure > 500 & pressure < 1000)
richardsc commented 7 years ago

Another demo that this is not working as expected:

data(argo)
as <- subset(argoGrid(argo), pressure > 500 & pressure < 1000)
plot(as.section(as), which='temperature')

image

dankelley commented 7 years ago

Thanks. I made a test file in the sister oce-issues repo, and it demonstrates the problem. I can have a look this afternoon, or you could look this morning -- either way, it's likely to be fairly simple.

dankelley commented 7 years ago

EDIT: this comment is superseded by others below, because the fix mentioned here was no good

I think this is fixed now in "develop" commit d974b747061b135b4287d2d271602331b74b74f8. I updated the issue code also. I didn't really look at the data to see if anything made sense -- all I did was to adjust the code. @richardsc -- can you look to see if the results seem okay in physical terms?

I suppose the issue is that we need to chop some lon+lat values because some stations might not have pressures in the given range ... maybe. As I say, though, I'm just looking at the code, hoping Clark can look at the results.

I think things are completely messed up because (relative to the previous comment by Clark) when I do

library(oce)
data(argo)
as <- subset(argoGrid(argo), pressure > 500 & pressure < 1000)
plot(as.section(as), which='temperature')

I get as shown below, which is nothing like what Clark gets.

screen shot 2017-11-05 at 8 37 13 am

dankelley commented 7 years ago

I'm going to look at this today, because the build tests are failing, so I've done something fundamentally wrong for this fix.

> plot(argo)
> plot(subset(argo, time > mean(time)))
> plot(subset(argo, longitude > mean(longitude)))
> plot(subset(argoGrid(argo), pressure > 500 & pressure < 1000), which=5)
Error in as.ctd(x@data$salinity, x@data$temperature, x@data$pressure,  : 
  lengths of salinity and longitude must match but they are 400 and 2230, respectively
Calls: plot -> plot -> .local -> as.ctd
Execution halted
dankelley commented 7 years ago

I think all is ok now in "develop" commit feec9b465314467c095239cc094d1b6fcec88233 (and I've added this last-mentioned problem to the issue test).

Well, I say that only because the tests I've created in the oce-issues repo work, and the build-time checks also work. I don't say it because I've actually looked at test cases in any detail on a scientific level... I'm hoping another person might do that.

Oh yeah, and now

library(oce)
data(argo)
as <- subset(argoGrid(argo), pressure > 500 & pressure < 1000)
png("1.png",width=7,height=3,unit="in",res=100,pointsize=10)
plot(as.section(argo), which='temperature')
dev.off()
png("2.png",width=7,height=3,unit="in",res=100,pointsize=10)
plot(as.section(as), which='temperature')
dev.off()

gives as follows (but note, again, that I'm not really looking in any detail at the results, but rather trying to smooth out wrinkles in code that I dried at too high a setting)

1.png

1

2.png

2

dankelley commented 7 years ago

I think this is fixed, so I'm closing it. Others should feel free to reopen it -- if so, I ask that the reopener supply a test so I'll know when I've addressed the existing/new problem.