-
Notifications
You must be signed in to change notification settings - Fork 37
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
Update enso notebook #832
Update enso notebook #832
Conversation
@lee1043 looking at the diff it does seem that some of the metrics have changed value slightly - is that expected given the change in datasets? |
@@ -801,11 +748,11 @@ | |||
" },\n", | |||
" \"metric\": {\n", | |||
" \"ERA-Interim\": {\n", | |||
" \"value\": 0.6404285425003315,\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lee1043 for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a couple of tests locally - ran your new notebook using the pmp_enso_tutorial_files.v20210823.txt data and ensometrics from conda-forge, and also ran the old notebook (in main) with pmp_enso_tutorial_files.v20210823.txt and ensometrics installed locally via pip. In both cases I am seeing the same statistics as the old notebook from the diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking. Let me try rerun the notebook in my local.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if I understand correctly, it seems as though the differences must be coming from a change in the obs. Do I have that right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gleckler1 I don't think it is caused by any OBS diff. Now the updated notebook and old notebook are using the same OBS (from the v20210823.txt), and differences in statistics are very small numbers (under point 9th digit), which I think it is negligible. I think the difference is coming from different machines were being used.
I am attaching a file that I got from below commend,
conda list > conda_list.txt
conda_list.txt
@acordonez could you try same on your env and see if there is any difference in versions of the PMP dependencies?
One possibility to keep the change in this PR to be more focused and traceable (most change in this PR is removing enso_metrics's manual installation part from the demo notebook), how about @acordonez uploads the new notebook ran from her local to this PR (or via new PR if that is easier)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gleckler1 @lee1043 Agree that it doesn't seem to be an issue with the obs, since I am getting the same results now that I got when we did the last round of updates. Differences due to machines sounds plausible to me. There are quite a few differences between my environment and Jiwoo's also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try running the notebook and pushing the results to this PR.
@acordonez thank you, I am closing this PR and will merge the new PR (#836). |
ENSO demo was updated -- manual installation process for the ENSO metrics by cloning it github repository has been removed.