search

STIMALiU / AdvRCourse

Course in advanced R programming
29 stars 31 forks source link

Possible change for test_brute_force_knapsack.R #13

Open ugurcanlacin opened 6 years ago

ugurcanlacin commented 6 years ago

The last test function in test_brute_force_knapsack.R has possible bug. The following two lines are my problem.

st <- system.time(bfk <- brute_force_knapsack(x = knapsack_objects[1:16,], W = 2000))

I have this output for the first line.

user system elapsed 0.348 0.000 0.348

And second line checks system column. However, it is zero everytime.

expect_true(as.numeric(st)[2] > 0.00)

So, I looked at the manual and the manual redirected me to proc.time function documentation.

It says The definition of ‘user’ and ‘system’ times is from your OS.

I use Ubuntu, searched in ubuntu forum and found very useful answer for it.

In the answer, he said:

So that's what your output means - there was no switch to kernel mode and CPU did not receive any interrupts from the program to do so. This also means that your code doesn't have anything that would require elevated privileges from CPU

In conclusion, I think we should change the column with user or elapsed. This is because we do not have any interruption during execution obviously. Probably processor is fast enough to not take interruption.

Here is information of my CPU: model name : Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz

MansMeg commented 6 years ago

Great! This is probably my mistake. I suggest you comment out/skip that test for now. I do not remember what OS is run on Travis? Is it failing on travis as well? You have any Travis log file you could point me to?

ugurcanlacin commented 6 years ago

I will comment out them. Thanks! I checked Travis logs and it is Ubuntu 14.04.5 LTS. I pushed the code and I didn't see error in Travis logs. This is probably because Travis CI creates virtual machine and it is not as fast as my own computer and has several interruptions.

ugurcanlacin commented 6 years ago

But still I think we should not check system time. It looks like out of purpose.

MansMeg commented 6 years ago

I agree. I will fix it when the course is done. Dont want to accidentally produce other errors by fixing this right now.

ugurcanlacin commented 6 years ago

Okey then. Thanks for your time. :+1:

MansMeg commented 6 years ago

Please leave this open and well fix it after the course!

rjkhan commented 6 years ago

i also stuck on this error @ugurcanlacin good to notify there you saved my day :)

gyommy15 commented 6 years ago

great help! thanks :+1: