-
Notifications
You must be signed in to change notification settings - Fork 23
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] Sea ice forecasting using the IceNet library #239
Comments
👋 @weiji14 @William-gregory we will conduct the review in this issue. |
Hi @weiji14 @William-gregory I am just checking in on the status of the review. Are there any obstacles or is there anything I can assist you with? |
Hi @annefou, apologies for the delay. I've just returned from vacation and will be getting on with the review this week. One initial point of note is that my environment build fails on my local machine (works fine on Binder). Am I doing something wrong? Reproducibility
Building the environment fails:
|
Hi @William-gregory, could I ask if you're using Windows? The library is supported on Linux (potentially Unix). |
Hi @bnubald, I'm attempting to run on MacOS |
On my side, I tried in mybinder.org and on my laptop (creating a local environment as you did). It works in mybinder.org out of the box but not on my laptop (MacOS too). But then I changed netCDF version to 1.6.5 and it seems to work. So maybe the version of netCDF could be changed to 1.6.5? |
There could be a potential issue with versions > 1.6.0 (icenet-ai/icenet#226). But, if its working, happy to consider. Else, maybe using conda for the netcdf4 install which has macos listed as supported. name: 67a1e320-7c47-4ea9-8df8-e868326bc90b
channels:
- conda-forge
- defaults
dependencies:
- libnetcdf
- pip
- python<3.12
- "tensorflow<2.16=cpu*"
- netcdf4=1.6.0
- pip:
- icenet @ git+https://github.com/icenet-ai/icenet@3654dc4954eca6d28e16b4876bd6538abd1f0c06
- ecmwflibs |
I managed to get the install working by using netCDF4 version 1.6.5. I also included jupyter in the dependencies in order to launch the notebook. I'm now getting an error on
|
Would this happen to be on an Apple Silicon system? Could you try using this as your conda env file please? name: 67a1e320-7c47-4ea9-8df8-e868326bc90b
name: icenet-edsbook
channels:
- conda-forge
- defaults
dependencies:
- cf-units
- libnetcdf
- netcdf4=1.6.0
- pip
- python<3.12
- "tensorflow<2.16=cpu*"
- pip:
- ecmwflibs
- icenet @ git+https://github.com/icenet-ai/icenet@3654dc4954eca6d28e16b4876bd6538abd1f0c06
- notebook |
Thanks @bnubald, that's done the trick. The notebook runs through fine now |
Review checklist for @weiji14Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide. Conflict of interest
Code of conduct an peer-review principles
General checks
Reproducibility
Pedagogy
Ethical
Other Requirements
|
Conflict of interest
Code of conduct an peer-review principles
General checks
Reproducibility
Pedagogy
Ethical
Other Requirements
|
@bnubald the two reviewers suggested a few changes. Would you have time to have a look and implement their suggestions? Many thanks. |
Thank you @weiji14, @William-gregory, @annefou, I will go through them, I might be a bit delayed (2-3 weeks) due to focusing on a conference. |
Hi @bnubald, just checking with you. How is it going? Any progress with the review? Many thanks. |
Hi @annefou, I've started going through the reviewer comments, will update soon. |
Thank you for reviewing this submission @weiji14. Apologies for submitting the changes in a bulk PR... I have gone through and resolved the comments in ReviewNB, only the last one on changing the licensing remains (@annefou, would you be able to advice please? comment is here). In addition to the inline comments in the notebook, please find below my responses to the checklist items (where actions were requested). Response to checklist items
Added
Added citation with DOI for ERA5, ORA5 and OSI SAF's sea-ice concentration data sources in the notebook.
Added a limitations section covering some aspects of this. Since the model in the demonstrator notebook includes only a few thousand parameters, and is only run for 3 epochs, comparison of results have been omitted. And, some aspects of this are better suited to a paper format which seems out of scope, I have referred to the original IceNet paper in this case. We are currently working on a paper covering the up-to-date IceNet library, but it is still a work in progress.
Added citations for python modules.
Added an acronym section in the header. |
Thank you for reviewing @William-gregory Please see below for my updates. Response to checklist items
|
Hi @William-gregory and @weiji14 Thanks for reviewing the notebook. Could you confirm that everything is OK so we can proceed to the publication? |
Hi @annefou, sorry for the slow response. I'm happy with the changes! Many thanks |
Hi @annefou, I see that the requested changes are still sitting in a Pull Request at eds-book-gallery/67a1e320-7c47-4ea9-8df8-e868326bc90b#6. Should those be merged into the |
Hi @weiji14, the review needs to be done in the pull request directly. Thanks. |
Ok, I've gone through the Pull Request, and am mostly satisfied with the changes. There are still some minor typos and parts that needs updating (see e.g. eds-book-gallery/67a1e320-7c47-4ea9-8df8-e868326bc90b#6 (comment)), but happy for this notebook to move on to the next stage otherwise 🚀 |
Thanks @William-gregory, @weiji14 for taking your time to go through this. I've updated and pushed latest changes to the Pull Request. |
Awesome! @acocac I think the publication process can now start. Thank you! |
@bnubald and team - Congratulations, your notebook is recommended for publication! 🚀 Huge thanks to our editor: @annefou and reviewers: @William-gregory, @weiji14 — your contributions make this possible 🙏 Next steps (optional for reviewers): @bnubald - I'll contact you for validating the post-print version and confirm a suitable date to announce the publication among the communication channels of the EDS book community. |
That's great! Thanks everyone (@annefou, @William-gregory, @weiji14, @acocac) for your help and support in getting this through. 👏 |
Notebook Review: Issue #236
Submitting author: @bnubald
Repository: https://github.com/eds-book-gallery/67a1e320-7c47-4ea9-8df8-e868326bc90b
Notebook idea issue: #221
Editor: @annefou
Reviewer: @weiji14 @William-gregory
Managing EiC: @acocac
Status
Reviewer instructions & questions
Hi 👋 @weiji14 & @William-gregory, please carry out your review in this issue by updating the checklist below.
As a reviewer, you contribute to the technical quality of the content published by our community.
Before the review, EiC checked if the submission fits the minimum requirements.
The quality of the proposed contribution can be assessed through scientific, technical and code criteria.
The reviewer guidelines are available here: https://edsbook.org/publishing/guidelines/guidelines-reviewers.html.
Any questions/concerns please let @annefou know.
Review checklist for @weiji14
Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide.
Conflict of interest
Code of conduct an peer-review principles
General checks
notebook.ipynb
) part of the notebook repository?Reproducibility
Pedagogy
Ethical
Other Requirements
Final approval (post-review)
Review checklist for @William-gregory
Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide.
Conflict of interest
Code of conduct an peer-review principles
General checks
notebook.ipynb
) part of the notebook repository?Reproducibility
Pedagogy
Ethical
Other Requirements
Final approval (post-review)
Additional instructions
Reviewer general comments are welcome on this REVIEW issue or directly to the notebook repository.
If you do the latter, you will find a Pull Request titled REVIEW where you can carry out the discussion with authors through ReviewNB, a third-party plugin in GitHub for displaying and commenting Jupyter Notebooks (see further details here).
In addition to ReviewNB, we suggest to explore or run the notebook in:
The text was updated successfully, but these errors were encountered: