alan-turing-institute / r-from-scratch

LearnR tutorial, and scripts to deploy on the cloud.
MIT License
1 stars 1 forks source link

Submission Correctness Tests (SCTs) not executing as expected #3

Open sgibson91 opened 5 years ago

sgibson91 commented 5 years ago

Problem

This issue was specific to the exercise at the end of Chapter 2 in which the student is asked to execute head and tail on the test data frame. The provided solution uses check_output() and matching a regular expression pattern in the student's output. While this worked in the DataCamp environment, transferring into the Shiny app caused the answer submission to throw the tailored missing message flag even though the provided solution was correct.

Solution

I replaced the check_output() call with ex() %>% check_function("head") %>% check_result() %>% check_equal() (same for tail) which checks for the call of head in both the solution and student, check_result() then executes the function in both environments, and check_equal() verifies that both outputs are equivalent. Manual tests verify that this method allows for both head(gender_data) and head(gender_data, 6) to produce correct answers, whereas head(gender_data, n) where n != 6 produces an incorrect answer.

Attempted Solution

I attempted to replace check_output() with check_output_expr() which takes the function name (as a string) as the argument and executes it in order to compare with the student answer, which I believed would be more robust as it was a direct execution rather than a comparison of regular expressions. However, this produced the same missing message as before. I'm still unclear on why this didn't work as digging into LearnR and testwhat to run tests proved difficult.

matthew-brett commented 5 years ago

For debugging, I found myself using the demo code on this page: https://datacamp.github.io/testwhat/ - so in this case, something like:

library(testwhat)
gender_data <- read.csv('http://bit.ly/gender-stats-data')
setup_state(sol_code = "head(gender_data)", stu_code = "head(gender_data, 7)")
ex() %>% check_function("head") %>% check_result() %>% check_equal()

which gives:

Error in check_that(is_true(eq_fun(stud_res, sol_res)), feedback = state$details) : 
  Check your call of `head()`. Running it again doesn't give the correct result. The result has 7 rows and 8 columns, while it should have 6 rows and 8 columns.
sgibson91 commented 5 years ago

I'm trying the following code chunk in the check section of this exercise.

gender_data <- read.csv('http://bit.ly/gender-stats-data')
setup_state(sol_code = "head", stu_code = "head(gender_data)")
ex() %>% check_output(.,
  '1    Afghanistan   4\\.95450  19961015094[[:space:]]+     161\\.1380',
  missing_msg='Did you display the value for "head(gender_data)"?')

I'm still getting the missing message when running the exercise with this style of SCT but I'm not getting any output at all regarding debugging. I also tried:

s <- ex() %>% check_output(.,
  '1    Afghanistan   4\\.95450  19961015094[[:space:]]+     161\\.1380',
  missing_msg='Did you display the value for "head(gender_data)"?')
print(str(s))

as per the demo code, but still nothing is printed. Similarly, playing around with the stu_code argument input didn't produce anything.

Interestingly when I update the sol_code argument to "head(gender_data)", I receive the message Warning: Error in head: object 'gender_data' not found even though it's defined a few lines above within the same section and defined in the student code too.

matthew-brett commented 5 years ago

Sorry to read at a run, but is it possible you got the student and solution fragments the wrong way round?

sgibson91 commented 5 years ago

I don't think so - I have them in order of exercise, hint, solution, then check.

matthew-brett commented 5 years ago

This worked as I expected, for me - is that not what you meant?

setup_state(sol_code = "head(gender_data)", stu_code = "head")
ex() %>% check_output(.,
   '1    Afghanistan   4\\.95450  19961015094[[:space:]]+     161\\.1380',
   missing_msg='Did you display the value for "head(gender_data)"?')

This gives:

Error in check_that(is_gte(num_hits, times), feedback = regex_state$details) : 
  Did you display the value for "head(gender_data)"?
sgibson91 commented 5 years ago

Copying and pasting that snippet gives me this message: Warning: Error in head: object 'gender_data' not found. This might be an issue with the migration to Shiny executing things differently than in the DataCamp environment?

matthew-brett commented 5 years ago

Just to confirm, copy/pasting this fragment:

library(testwhat)
gender_data <- read.csv('http://bit.ly/gender-stats-data')
setup_state(sol_code = "head(gender_data)", stu_code = "head")
ex() %>% check_output(.,
   '1    Afghanistan   4\\.95450  19961015094[[:space:]]+     161\\.1380',
   missing_msg='Did you display the value for "head(gender_data)"?')

in R, runs OK until the ex() line, when you get the error 'gender_data' not found? That code works fine for me, in R.

Or you mean, that in LearnR, that's what you get? If in LearnR, is it possible the relevant code setup did not get run? https://rstudio.github.io/learnr/exercises.html#exercise_setup

sgibson91 commented 5 years ago

The fragment runs fine until the setup_state line, where I get the error "gender_data" not found. If I change to sol_code = "head", I do not get this error but I am getting the exercise marked as incorrect when it should be correct.

I'm copy/pasting into LearnR.Rmd, locally executing the Shiny app and pressing Run Code and Submit Answer every time so the code setup should be happening as far as I'm aware.

I changed the formatting slightly as in your DataCamp-hosted files, labels such as @solution where used whereas the notation {r solution} is used here. However, anything that was marked as pre-exercise, I made sure was available in exercise chunks here. That's why I'm struggling to understand why the code cannot find gender_data because it is in fact loaded 3 times: first in the exercise chunk, second in the solution chunk, and lastly in the check itself (though this isn't necessary for the solution I have given in the first comment).

matthew-brett commented 5 years ago

Aha - can you point to the failing code somewhere in a branch?

sgibson91 commented 5 years ago

Here's the failing code.

matthew-brett commented 5 years ago

I can't make the regex work - but I can make the checks run without an error finding gender_data, with some setup code:

https://github.com/alan-turing-institute/r-from-scratch/pull/4

sgibson91 commented 5 years ago

This looks promising. Can we make solution and check chunks reference the get-gender-data chunk too? I'd like to eliminate all unnecessary loads of the data where possible.

Ultimately, I don't think we should worry too much about getting regex working as I think ex() %>% check_function("head") %>% check_result() %>% check_equal() is probably a more robust method of checking the correct number of lines have been printed, rather than just checking the first line. So perhaps we can just leave it as ex() %>% check_function("head") %>% check_result() %>% check_equal() as in #2 ?

matthew-brett commented 5 years ago

I did try adding `exercise.setup="get-gender-data" to the solution, but it didn't work. So, no, annoyingly, I don't think there is a way to avoid the duplication. I've just raised an issue with LearnR : https://github.com/rstudio/learnr/issues/202

matthew-brett commented 5 years ago

Yes, fine to get rid of the regexps. Maybe avoid the duplication of ex() with:

state = ex();
check_function(state, "head") %>% check_result() %>% check_equal()
check_function(state, "tail") %>% check_result() %>% check_equal()
sgibson91 commented 5 years ago

Ok, hopefully something comes out of that issue. But if not, the tutorial runs as is.

Here's the amended SCT avoiding duplicates of ex().

I've also gone through and implemented the setup code for the data, so from Chapter 2 it's pre-loaded in the student chunks.

matthew-brett commented 5 years ago

Sorry - the other thing I was going to say was, that I noticed the checks are pretty slow, and I think that's because it's downloading the data each time. I think it would be worth committing the data to the repository, and making the setup chunk load the local copy. What do you think?

sgibson91 commented 5 years ago

It might help. The problem is if we can't link the solution/check chunks to the setup chunk, then they're still going to be loading the data - just from the repo rather than the website. Though I guess that'll probably still be quicker than from the website?

matthew-brett commented 5 years ago

I think it will be massively quicker, doing a local load. But I haven't timed it. We could also save the data to an R binary format, that might be quicker still. But I'm guessing that a local load will be so fast it will be difficult to detect the difference.

sgibson91 commented 5 years ago

Ok, I'll start a new branch to develop the local load.

matthew-brett commented 5 years ago

Yes - waaaay faster - I get this:

> system.time(gender_data <- read.csv('http://bit.ly/gender-stats-data'))
   user  system elapsed 
  0.017   0.002   0.433 
> system.time(gender_data <- read.csv('http://bit.ly/gender-stats-data'))
   user  system elapsed 
  0.018   0.002   0.679 
> system.time(gender_data <- read.csv('http://bit.ly/gender-stats-data'))
   user  system elapsed 
  0.018   0.003   0.677 
> system.time(gender_data <- read.csv('http://bit.ly/gender-stats-data'))
   user  system elapsed 
  0.017   0.002   0.437 

Then I do:

curl -LO http://bit.ly/gender-stats-data

and re-run in R:

> system.time(gender_data <- read.csv('gender-stats-data'))
   user  system elapsed 
  0.002   0.001   0.002 
> system.time(gender_data <- read.csv('gender-stats-data'))
   user  system elapsed 
  0.002   0.000   0.002 
> system.time(gender_data <- read.csv('gender-stats-data'))
   user  system elapsed 
  0.002   0.000   0.002 
> system.time(gender_data <- read.csv('gender-stats-data'))
   user  system elapsed 
  0.002   0.001   0.002