Closed bilalderouich closed 5 years ago
@pradal @christian34 @Emblanc I start writing the test, can you validate if it's a good reference test and review the pull request
Great ! I will need to take a closer look at the test but, for starters, I can say that, given what I know about WALTer, I think this test should probably be done on a whole simulation (i.e nbj = 325 and not 165)
When we start the simulation with 325 days, we see that it is slowed down, almost stopped.
When we start the simulation with 325 days, we see that it is slowed down, almost stopped.
It is to be expected, because caribu starts on day 162 of the test_simulation, which means that the first 161 days of the simulation are very quick and the following days are slower to compute. Moreover, the files to compare will be larger, so the comparison will take longer as well. Thus, the test will be longer with 325 days, but it is important to do it on a whole simulation because changes in the results often happen after a few days running caribu.
When I run nosetests on my computer with this PR I get an error :
============================================= FAIL: test.test_project.test_project1
Traceback (most recent call last): File "/home/emmanuelle/miniconda2/envs/walter/lib/python2.7/site-packages/nose/case.py", line 197, in runTest self.test(*self.arg) File "/home/emmanuelle/WALTer_Dvpt/test/test_project.py", line 15, in test_project1 assert str(cwd) == whereIam, str(cwd) + ' / '+ whereIam AssertionError: /home/emmanuelle/WALTer_Dvpt/test/same / /home/emmanuelle/WALTer_Dvpt/test
This is probably because there is no "p.deactivate()" at the end of the test_same_result.
Note that the global test does not pass on my machine, probably due to too strict comparison (default precision). I will investigate to confirm asap .
Actually there is a difference up to 200 between PAR values in windows and linux simulation. I adapt the test to be tolerant for theses column up to this value. @christian34 : can you check that the test pass ? @Emblanc @chrlecarpentier : did you notice that difference earlier ?
Ok, after a typo error fix (the old strict test was executed), it works ! I think we should try, in future version of walter that improve PAR computation, to progressively decrease this value of tolerance for PAR comparison in global test (may be the clean_geometry branch will remove part of this quite large platform dependant discrepancy ?)
Actually there is a difference up to 200 between PAR values in windows and linux simulation. I adapt the test to be tolerant for theses column up to this value. @christian34 : can you check that the test pass ? @Emblanc @chrlecarpentier : did you notice that difference earlier ?
I have never used WALTer on Windows so I could not detect this difference. I am very surprised to learn that there are such variations between os. Do you have any idea where it could come from ? Is the intercepted PAR really the only difference between the simulation outputs of windows and linux ?
I have never used WALTer on Windows so I could not detect this difference. I am very surprised to learn that there are such variations between os. Do you have any idea where it could come from ? Is the intercepted PAR really the only difference between the simulation outputs of windows and linux ?
Yes the only difference is for columns with 'PAR' in their name (all the other columns are tested for strict identity) My guess for the bug is Caribu processing of 'very thin' triangles, that return sometimes strange values (I am working on a solution for that). These values may be related to difference in floating point precision between os ?. I have found that the relative error between windows and the reference simulation is the highest for very thin cylindric primitives (relative error for blades is below 5%, up to 10% for sheath end ears and up to 20% for peduncles). Note that this occur only for some primitives, as shown on the boxplots:
With Bilal, we make a windows/linux comparison of the non_regression test on the clean_geometry branch. Things are much better than on the master: It somehow confirm that the problem comes from caribu processing of small triangle, that are more frequent and smaller in master. I propose that Bilal adapt the test to the clean geometry situation (ie we tolerate error up to 2% on PAR values). For now the test will fail on window, but, once the clean_geometry will be merged and the reference updated, it will pass. @Emblanc @pradal Is it okay for you ?
I propose that Bilal adapt the test to the clean geometry situation (ie we tolerate error up to 3-5 % on PAR values). For now the test will fail on window, but, once the clean_geometry will be merged and the reference updated, it will pass. @Emblanc @pradal Is it okay for you ?
That is good news ! I think it is reassuring regarding the reliability of our results. I agree with you on the adaptation of the test to the clean geometry situation.
It is OK for me
The sim_scheme_ref.csv can be used instead of the sim_scheme_test.csv
Setup none regression test