Skip to content

new rcpp code#1

Merged
andrew-MET merged 5 commits intoharphub:masterfrom
adeckmyn:master
Apr 11, 2019
Merged

new rcpp code#1
andrew-MET merged 5 commits intoharphub:masterfrom
adeckmyn:master

Conversation

@adeckmyn
Copy link
Copy Markdown
Contributor

ecoval and roc are now Rcpp codes that return the results in a similar form as the old functions.
To call them, I only modified the harp_ecoval and harp_roc functions.
Results should be numerically identical, and much faster.

@andrew-MET andrew-MET self-assigned this Apr 9, 2019
@andrew-MET andrew-MET added the enhancement New feature or request label Apr 9, 2019
@andrew-MET andrew-MET self-requested a review April 9, 2019 11:36
Copy link
Copy Markdown
Collaborator

@andrew-MET andrew-MET left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add SystemRequirements: C++11 to DESCRIPTION otherwise the lambda function in roc.cpp doesn't work on all platforms (it didn't compile without it on one system I tested on).

Also I'm getting different results for economic value in a small number of occasions. Can you try the following on ecgate:

library(harpIO)
library(harpPoint)

fctable_dir  <- "/scratch/ms/no/fa1m/harp_output/FCTABLE"
obstable_dir <- "/scratch/ms/no/fa1m/harp_output/OBSTABLE"
first_fcst   <- 2017053000   
last_fcst    <- 2017053100
fcst_freq    <- "1d"
fcst_models  <- c("model1", "model2", "model3")
param        <- "T2m"
lead_times   <-  seq(0, 36, 3)

verif_T2m <- ens_read_and_verify(
  start_date = first_fcst, 
  end_date   = last_fcst,
  parameter  = "T2m", 
  fcst_model = fcst_models,
  fcst_path  = fctable_dir,
  obs_path   = obstable_dir,
  lead_time  = lead_times,
  thresholds = seq(285, 305, 5)
)

with both the current version and the PR.

I find that verif_T2m$ens_threshold_scores$economic_value[c(22,137)] are different before and after the PR. I think the PR actually corrects an error in the verification package version, which gives NAs, but can you confirm?

Rows 22 and 137 are:

 mname == "model3", leadtime == 3,  threshold == 290
 mname == "model2", leadtime == 27, threshold == 290

@adeckmyn
Copy link
Copy Markdown
Contributor Author

I added the C++11 requirement. The difference in ecoval seems to be for the case when all /forecasts/ are the same (either FALSE or TRUE). My code only gives NA when all /observations/ are either T or F (because you get numerical problems in that case). A constant forecast may be rather useless, but I don't think it causes the definition of economic value to fail. In the verification package that is what happens.

@andrew-MET andrew-MET merged commit 797f791 into harphub:master Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants