-
Notifications
You must be signed in to change notification settings - Fork 7
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
Hard coded exception in 'sw_SiteClimate_Ambient' for year 1942 #17
Comments
Interesting, I'm not sure why it's there. I don't think it's my code. Maybe someone was writing a test case, or something? Anyway, I'll delete it since it doesn't look like it has any effect on the Wrapper. |
We could not determine the original purpose of the piece of code and so it has been removed
In retrospect, I think I am the one who hardcoded the 1942 exception into the sw_SiteClimate_Ambient function. I ran into the 1942 'problem' again today. For some reason, in 1942, the month_forEachDoy statement, only creates a sequence that is a length of 364, and ends on 12-30-1942 (excludes 12-31-1942). The hardcoded exception added an additional day-month to account for 12-31-1942. Do you have a more elegant solution to fix this? I will send along weather files in a separate file so you can see for yourself. |
Replicate problem described by Caitlin for year 1942sessionInfo()
## Load weather data in question
weath <- Rsoilwat31::getWeatherData_folders(
LookupWeatherFolder = ".", weatherDirName = "Weather_SheepStation",
filebasename = "weath", startYear = 1926, endYear = 2015)
# Check structure
str(weath[["1942"]])
wdims <- sapply(weath, function(x) dim(x@data))
isLeapYear <- function(y) {
#from package: tis
y %% 4 == 0 & (y %% 100 != 0 | y %% 400 == 0)
}
identical(as.vector(wdims[1, ]), ifelse(isLeapYear(1926:2015), 366L, 365L))
Sys.timezone()
I tested this for "Europe/Berlin" and "Europe/Zurich". I copy-pasted results below for "Europe/Berlin": # Code from 'sw_SiteClimate_Ambient'
weatherList <- weath
year.start <- 1926
year.end <- 2015
do.C4vars <- FALSE
sw.weather.suffix <- as.numeric(names(weatherList))
itemp <- year.start <= sw.weather.suffix & year.end >= sw.weather.suffix
years <- sw.weather.suffix[itemp]
no.yrs <- length(years)
y <- which(years == 1942)
x <- Rsoilwat31::get_swWeatherData(weatherList, years[y])@data[, c("Tmax_C", "Tmin_C", "PPT_cm"), drop = FALSE]
temp.dailyTempMean <- rowMeans(x[, c("Tmax_C", "Tmin_C")])
temp_seq <- seq(from = as.POSIXlt(paste0(years[y], "-01-01")),
to = as.POSIXlt(paste0(years[y], "-12-31")),
by = "1 day")
month_forEachDoy <- as.POSIXlt(temp_seq)$mon + 1
print(data.frame(temp_seq, month_forEachDoy)[c(305:308, 360:365), ])
For timezone "Europe/Berlin", I get
==> there is no date for Dec 31, 1942 even though there are 365 entries print(temp_seq[305:308])
For timezone "Europe/Berlin", I get:
==> date Nov 2, 1942 occurs twice due to switch from CEST to CET Explanation of issue:Note: see Regression in strptime: See History of Daylight Saving Time — DST: ==> If I run the code on my system where Sys.timezone() returns "Europe/Zurich", ==> Thus, issue 17 should appear for US, Canadian, and German/Hitler time zones Suggested solution: fix time zone to UTC during time sequence calculationtemp_seq2 <- seq(from = ISOdate(years[y], 1, 1, tz = "UTC"),
to = ISOdate(years[y], 12, 31, tz = "UTC"),
by = "1 day")
month_forEachDoy2 <- as.POSIXlt(temp_seq2)$mon + 1
print(data.frame(temp_seq2, month_forEachDoy2)[c(305:308, 360:365), ])
print(temp_seq2[305:308])
|
How interesting! Who knew that programming could lead to a quick history On Wed, Sep 28, 2016 at 2:52 AM, daniel [email protected] wrote:
Caitlin Andrews |
The function 'sw_SiteClimate_Ambient' has a hard-coded exception for the year 1942:
- If the year is 1942, then an additional day is added to the sequence 'month_forEachDoy'
I don't see the reason why that this is done. It appears that the code believes that the year 1942 is a leap year and that the input argument 'weatherList' would treat it as non-leap year? In any case, 1942 was not a leap year.
Please explain code in a comment or remove.
Thanks, Daniel
The text was updated successfully, but these errors were encountered: