Skip to content
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

REVIEW #4

Closed
wants to merge 1 commit into from
Closed

REVIEW #4

wants to merge 1 commit into from

Conversation

acocac
Copy link
Member

@acocac acocac commented May 23, 2023

Hi 👋

Thanks authors and reviewers to contribute to the Climate Informatics 2023 Reproducibility Challenge ✨

The review PR aims to facilitate discussion how to improve the notebook via ReviewNB, a third-party plugin in GitHub for displaying and commenting Jupyter Notebooks. Find further details here.

Authors: please reply reviewers comments in a timely manner and push changes in this PR. You'll see ReviewNB easily renders Jupyter Notebook rich diffs than GitHub (see here).

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

github-actions bot commented May 23, 2023

Binder 👈 Test this PR on Binder

notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
@NHomer-Edi
Copy link

NHomer-Edi commented May 25, 2023

I have commented on the ReviewNB, but here are some more general comments:
Well executed and documented (inline and markdown) notebook, which has a clear purpose and runs well locally. Clearly fits within the scope of the Environmental DS Book.

  • Notebook doesn't successfully run on Binder, for me (not the author's fault) EDIT: I did eventually get the notebook to run on Binder
  • References to data sources and acronyms should be added
  • Some more discussion around your choice of default parametrs (e.g. LR, batch sizes etc.) would be beneficial
  • Add some more information about the limitations of your methods (albeit there is little discussion of limitations in the paper) ( Add truthful and clear limitations of the analysis (and potential biases in the data and/or tools) to the Notebook #6 )
  • Conclusion section needed ( Add conclusion to Notebook #5 )
  • The benefit of notebooks over papers is that they allow for more interactive plotting so that data can be explored more thoroughly. If time permits, it would be great to add interactivity to the plots.

notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
Copy link
Member Author

@acocac acocac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The notebook is good to go -- the reviewed version is available in the Notebook-Final branch!

notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
notebook.ipynb Show resolved Hide resolved
@acocac acocac closed this Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants