-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Different test results on pr-261-MH branch #265
Conversation
@martinholmer I'm using version 0.22.0 of pandas, which is causing an error after PR #262 because |
@andersonfrailey, One possibility not mentioned earlier in #265, is that the two raw PUF and CPS files I have on my computer are not the same as what you have on your computer. Here is the byte size and MD5 checksum for the two raw files:
Do you get the same size and checksum values on your computer? |
@andersonfrailey said:
Can you upgrade to 0.23.0 to see if you get the results I'm getting. Also, look at the |
I get the same md5 results as you, @martinholmer. Just updated my pandas version and am running everything again. |
@andersonfrailey said:
Here's my report: when I upgraded from pandas 0.23.0 to 0.23.3 I got slightly different results. |
@andersonfrailey, continuing to use pandas 0.23.3 (the latest version), I find that |
@andersonfrailey, if you glance through pandas issue 4588, which was opened in 2013 and was just recently "resolved" in 0.23.0 with bug fixed in 0.23.1, you can see a possible reason why we are getting different results with pandas 0.22 and 0.23.1+. The title of that issue is The discussion of this issue (bug report) eventually came up with the idea of adding the @hdoupe had an experience like this with pandas last year, in which they broke our code (and many other people's code) when they introduced without any notice a change in If you can get the same results as I get when you use pandas 0.23.3 (the latest version), then I think we should consider changing the environment and conda recipe to |
@martinholmer said:
I think that's sensible. I'll update to the latest version of Pandas and compare results. |
The tip of pull request #265 uses pandas 0.23.3 (the latest version) and changes the The new This pull request is not for merging, but put on GitHub in order to facilitate comparison of results generated on other local computers. |
@martinholmer, even with the new Pandas version I'm getting different results than you, but they haven't changed with regards to PR #261. I'm going to push up all my changes to #261. Maybe there's a commit somewhere that didn't get pushed before you started testing. I'll also start #261 from scratch and see what happens. Shouldn't take very long to get initial results. |
Even starting #261 from scratch I get the same results. @martinholmer can you try updating your branch and running everything again? |
[UPDATE: Puts resulting files in a directory on your regular file system] Here are the results that I got:
Here's the Dockerfile:
To build, set your directory like so (
Build the image:
Run the container:
Run the following commands:
I hope this helps. I got the exact checksum locally. Perhaps, I did something wrong in this process. The upside is that everyone can see exactly how the environment and files were set up and any errors can be fixed relatively painlessly. |
@hdoupe, Thanks for all the docker work. The checksums are a good start, but it seems to me the next step is to compare |
Sure, I'll update the comment above to show how to transfer the files from the container back to your local host file system. |
I updated my comment with a command to put files in the regular file system. |
I created a version of |
@andersonfrailey said:
What does that mean? You said this morning that @hdoupe had generated a Is the docker-generated |
One way to think about it is that the resulting files generated by docker (I generated the same ones without docker, too) is the "correct" one if you agree with the steps used to create the docker image and run the file creation scripts. I say "correct" because the process for setting up the environment and running the scripts is much more transparent--you can see each and every step taken. Further, you don't have to worry about other factors that could be affecting your results like what was in your conda build cache when the environment was built. The next step that I would take would be to try to recreate the files without docker using a process similar to that of the docker container process outlined above. If you can't, then there may be some local environment differences that are affecting the results. If you are successful in doing so and you agree that the steps taken to check out the correct git branch, merge in the correct changes, use the correct files, install the correct packages, and run the correct scripts; then that is probably the correct file. I'm happy to continue this discussion. If there are any questions or misunderstandings about docker, I am happy to address them or reference the appropriate documentation. |
@hdoupe and @andersonfrailey, The use of docker to figure out why we all got different |
@hdoupe, I don't understand your use of
What's the |
@martinholmer it's my understanding that the Docker container is creating an environment with just the latest versions of the packages specified in
It's possible that overtime the I'm going to try deleting and recreating my |
@andersonfrailey said:
So, you're saying that @hdoupe generated exactly the same |
@martinholmer that's correct. Here is the MD5 for that file: |
Good news! After deleting and recreating my Now that we've been able to produce the same file across two machines and in a docker container I feel confident in saying the issue was with packaging. @martinholmer would you mind running |
@andersonfrailey said:
I would not be surprised if this turns out to be true. But I think you're not giving enough consideration to another possibility: that the Matching code (and there is a lot of it) has been written in a way that does not respect the differences between Python 2.7 and Python 3.6.
But when I add this line at the top of each Python file in the Matching subdirectory, I generate a different
Here is the info on my newly generated (under Python 2.7)
Why is the
It looks to me that the The fact that adding |
@andersonfrailey said:
Here is what I have on my computer:
|
After a phone conversation with @martinholmer, I modified the matching scripts to ensure that all division in the matching scripts results in floating point numbers whether you're using Python 2.7 or 3.6. Once I did this I was able to produce the same file using both 2.7 and 3.6 and each is the same as what we've been getting using the docker container. It looks like between the working with a clean environment and the changes I made at the suggestion of @martinholmer to ensure the same results in 2.7 and 3.6 have solved our replicability problem. For a little more detail, I looked at the differences between the environment I used and the one @martinholmer uses. Here are the only packages using different versions:
I'm going to run all of the make files overnight to get a new PUF, new weights, and new ratios and will update PR #261 afterwards for others to test. |
Conversation has continued in #261. |
I'm getting different PUF test_data results on a local branch that includes the code changes in PR #261.
Here is what I've done:
make git-pr N=261
git merge master
to include recent master changes in the local copy of PR Include Dependent Filers in PUF #261make puf_data/cps-matched-puf.csv
make puf_data/puf.csv
test_pufcsv_data
puf_agg_actual.txt
topuf_agg_expect.txt
@anderson, note the maximum
age_spouse
difference and others.Have I done something wrong? Why are the test results different?
Can it be that the
float_format='%.2f'
used to write thecps-matched-puf.csv
file makes a difference?What version of pandas are you using? I'm using 0.23.0 pandas.
Do the
test_pufcsv_data
results I'm getting look "fishy" in any way?