openalea / WALTer

WALTer is a 3D FSPM Wheat model
Other
4 stars 8 forks source link

Combi_params error #72

Closed Emblanc closed 5 years ago

Emblanc commented 5 years ago

Changing the function to read the sim_scheme.csv file and adding a test for the combi_param bug

Emblanc commented 5 years ago

This pull_request solves issue #71 It is ready to be merged

Emblanc commented 5 years ago

I have done more tests, and I think this time, the pull-request is really ready to be merged

christian34 commented 5 years ago

Did you try the float_precision='high' in read_csv ? Or may be this is due to wrong inference of data type (if the input file is big, only a subset is used by pandas to infer type). In the later case, using engine='c' and low_memory=False force pandas reading the whole file, and then try to convert to int64, or if failing to float64 (which is what you are doing in the proposed solution)

christian34 commented 5 years ago

The test is probably wrong : it does not raise the bug on my machine . It could also be simplified using the 'test_project' syntax (instead of the 'test_command_line' syntax) and assertion can be made on counting rows of p.combi_params (which avoid recopy of read_itable in test file)

Emblanc commented 5 years ago

The float_precision='high' in read_csv seems to be working, thank you ! (And it is much simpler than the current proposed solution). I am going to run a few more tests before I change the file.

I will also put the test in the test_project file, using the correct syntax. Regarding the failure to reproduce the bug, I am not sure what causes it, but I have noticed it too. The bug may or may not occur depending on the computer (I think that it might be related to the version of a library maybe ?)

I have observed the following results (I do not know if it is relevant) :

computer1 : pandas 0.21.1 -> the test does not raise the bug computer2 : pandas 0.23.4 -> the test raises the bug