From 324c27273427d18f99e6d9a129ddd5494f6bfbe6 Mon Sep 17 00:00:00 2001 From: aho Date: Wed, 8 Apr 2020 16:41:42 +0200 Subject: [PATCH 1/2] Bugfix for un-reorder_lon + transform + lon from big to small. Add corresponding unit test. --- R/Start.R | 27 +++- .../test-Start-line_order-consistency.R | 143 ++++++++++++++++++ 2 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 tests/testthat/test-Start-line_order-consistency.R diff --git a/R/Start.R b/R/Start.R index 9a808b1..f02d867 100644 --- a/R/Start.R +++ b/R/Start.R @@ -2208,13 +2208,26 @@ print("-> SELECTORS REQUESTED BEFORE TRANSFORM.") subset_vars_to_transform[[var_with_selectors_name]] <- Subset(sub_array_of_values, inner_dim, sub_array_of_fri) } -## NOTE: Remove 'crop' from transform_params if no reorder. It causes error. -## But 'crop' has effect on reorder cases... need further investigation - if (is.null(dim_reorder_params[[inner_dim]])) { - if ('crop' %in% names(transform_params)) { - transform_params <- transform_params[-which(names(transform_params) == 'crop')] - } - } + +# Change the order of longitude crop if no reorder + from big to small. +# cdo -sellonlatbox, the lon is west, east (while lat can be north +# to south or opposite) +# NOTE: The case not considered: if lon reorder(decreasing = T) +# It doesn't make sense, but if someone uses it, here should +# occur error. +# Before changing crop, first we need to find the name of longitude. +# NOTE: The potential bug here (also the bug for CDORemapper): the lon name +# is limited (only the ones listed in .KnownLonNames() are available. + known_lon_names <- s2dverification:::.KnownLonNames() + lon_name <- names(subset_vars_to_transform)[which(names(subset_vars_to_transform) %in% known_lon_names)[1]] + + if ('crop' %in% names(transform_params) & is.null(dim_reorder_params[[inner_dim]])) { + if (transform_params$crop[1] > transform_params$crop[2]) { + tmp <- transform_params$crop[1] + transform_params$crop[1] <- transform_params$crop[2] + transform_params$crop[2] <- tmp + } + } transformed_subset_var <- do.call(transform, c(list(data_array = NULL, variables = subset_vars_to_transform, diff --git a/tests/testthat/test-Start-line_order-consistency.R b/tests/testthat/test-Start-line_order-consistency.R new file mode 100644 index 0000000..3649240 --- /dev/null +++ b/tests/testthat/test-Start-line_order-consistency.R @@ -0,0 +1,143 @@ +context("Start() line order consistency check") + +test_that("1. lon and lat order", { + + variable <- "tas" + obs.path <- "/esarchive/recon/ecmwf/era5/monthly_mean/tas_f1h/tas_$file_date$.nc" + dates_file <- "201702" + + lats.min <- -90 + lats.max <- 90 + lons.min <- 0 + lons.max <- 360 + + dat1 <- Start(dat = obs.path, + var = variable, + file_date = dates_file, + latitude = values(list(lats.min, lats.max)), + longitude = values(list(lons.min, lons.max)), + synonims = list(latitude=c('lat','latitude'), + longitude=c('lon','longitude')), + transform = CDORemapper, + transform_params = list( + grid = 'r360x181', + method = 'con', + crop = c(lons.min,lons.max,lats.min,lats.max) + ), + transform_vars = c('latitude', 'longitude'), + return_vars = list(latitude = 'dat', + longitude = 'dat', + time = 'file_date'), + retrieve = T) + + dat2 <- Start(dat = obs.path, + var = variable, + file_date = dates_file, + longitude = values(list(lons.min, lons.max)), + latitude = values(list(lats.min, lats.max)), + synonims = list(latitude=c('lat','latitude'), + longitude=c('lon','longitude')), + transform = CDORemapper, + transform_params = list( + grid = 'r360x181', + method = 'con', + crop = c(lons.min,lons.max,lats.min,lats.max) + ), + transform_vars = c('latitude', 'longitude'), + return_vars = list(latitude = 'dat', + longitude = 'dat', + time = 'file_date'), + retrieve = T) + + expect_equal( + length(attr(dat1, 'Variables')$dat1$latitude), + length(attr(dat2, 'Variables')$dat1$latitude) + ) + expect_equal( + length(attr(dat1, 'Variables')$dat1$longitude), + length(attr(dat2, 'Variables')$dat1$longitude) + ) +}) + + +test_that("2. dim length check: with/out reorder", { + + + dat1 <- Start(dat = obs.path, + var = variable, + file_date = dates_file, + latitude = values(list(lats.min, lats.max)), + longitude = values(list(lons.min, lons.max)), + synonims = list(latitude=c('lat','latitude'), + longitude=c('lon','longitude')), + transform = CDORemapper, + transform_params = list( + grid = 'r360x181', + method = 'con', + crop = c(lons.min,lons.max,lats.min,lats.max) + ), + transform_vars = c('latitude', 'longitude'), + return_vars = list(latitude = 'dat', + longitude = 'dat', + time = 'file_date'), + retrieve = T) + + dat2 <- Start(dat = obs.path, + var = variable, + file_date = dates_file, + latitude = values(list(lats.min, lats.max)), + latitude_reorder = Sort(), + longitude = values(list(lons.min, lons.max)), + # longitude_reorder = CircularSort(0, 360), + synonims = list(latitude=c('lat','latitude'), + longitude=c('lon','longitude')), + transform = CDORemapper, + transform_params = list( + grid = 'r360x181', + method = 'con', + crop = c(lons.min,lons.max,lats.min,lats.max) + ), + transform_vars = c('latitude', 'longitude'), + return_vars = list(latitude = 'dat', + longitude = 'dat', + time = 'file_date'), + retrieve = T) + + dat3 <- Start(dat = obs.path, + var = variable, + file_date = dates_file, + latitude = values(list(lats.min, lats.max)), + longitude = values(list(lons.min, lons.max)), + longitude_reorder = CircularSort(0, 361), + synonims = list(latitude=c('lat','latitude'), + longitude=c('lon','longitude')), + transform = CDORemapper, + transform_params = list( + grid = 'r360x181', + method = 'con', + crop = c(lons.min,lons.max,lats.min,lats.max) + ), + transform_vars = c('latitude', 'longitude'), + return_vars = list(latitude = 'dat', + longitude = 'dat', + time = 'file_date'), + retrieve = T) + + expect_equal( + length(attr(dat1, 'Variables')$dat1$latitude), + length(attr(dat2, 'Variables')$dat1$latitude) + ) + expect_equal( + length(attr(dat1, 'Variables')$dat1$longitude), + length(attr(dat2, 'Variables')$dat1$longitude) + ) + expect_equal( + length(attr(dat1, 'Variables')$dat1$latitude), + length(attr(dat3, 'Variables')$dat1$latitude) + ) + expect_equal( + length(attr(dat1, 'Variables')$dat1$longitude), + length(attr(dat3, 'Variables')$dat1$longitude) + ) + +}) -- GitLab From 68cca4636386e7e96df102709cc1c9704396048a Mon Sep 17 00:00:00 2001 From: aho Date: Wed, 8 Apr 2020 17:40:33 +0200 Subject: [PATCH 2/2] Bugfix when crop = TRUE/FALSE --- R/Start.R | 19 +++++++++++-------- .../test-Start-line_order-consistency.R | 6 +++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/R/Start.R b/R/Start.R index f02d867..1cf059c 100644 --- a/R/Start.R +++ b/R/Start.R @@ -2212,20 +2212,23 @@ print("-> SELECTORS REQUESTED BEFORE TRANSFORM.") # Change the order of longitude crop if no reorder + from big to small. # cdo -sellonlatbox, the lon is west, east (while lat can be north # to south or opposite) -# NOTE: The case not considered: if lon reorder(decreasing = T) -# It doesn't make sense, but if someone uses it, here should -# occur error. + # Before changing crop, first we need to find the name of longitude. # NOTE: The potential bug here (also the bug for CDORemapper): the lon name # is limited (only the ones listed in .KnownLonNames() are available. known_lon_names <- s2dverification:::.KnownLonNames() lon_name <- names(subset_vars_to_transform)[which(names(subset_vars_to_transform) %in% known_lon_names)[1]] - if ('crop' %in% names(transform_params) & is.null(dim_reorder_params[[inner_dim]])) { - if (transform_params$crop[1] > transform_params$crop[2]) { - tmp <- transform_params$crop[1] - transform_params$crop[1] <- transform_params$crop[2] - transform_params$crop[2] <- tmp +# NOTE: The cases not considered: (1) if lon reorder(decreasing = T) +# It doesn't make sense, but if someone uses it, here should +# occur error. (2) crop = TRUE/FALSE + if ('crop' %in% names(transform_params) & var_with_selectors_name == lon_name & is.null(dim_reorder_params[[inner_dim]])) { + if (is.numeric(class(transform_params$crop))) { + if (transform_params$crop[1] > transform_params$crop[2]) { + tmp <- transform_params$crop[1] + transform_params$crop[1] <- transform_params$crop[2] + transform_params$crop[2] <- tmp + } } } diff --git a/tests/testthat/test-Start-line_order-consistency.R b/tests/testthat/test-Start-line_order-consistency.R index 3649240..1fe98ea 100644 --- a/tests/testthat/test-Start-line_order-consistency.R +++ b/tests/testthat/test-Start-line_order-consistency.R @@ -1,16 +1,16 @@ context("Start() line order consistency check") -test_that("1. lon and lat order", { - variable <- "tas" obs.path <- "/esarchive/recon/ecmwf/era5/monthly_mean/tas_f1h/tas_$file_date$.nc" dates_file <- "201702" lats.min <- -90 lats.max <- 90 - lons.min <- 0 + lons.min <- 0 lons.max <- 360 +test_that("1. lon and lat order", { + dat1 <- Start(dat = obs.path, var = variable, file_date = dates_file, -- GitLab