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]: GeoBO: Python package for Multi-Objective Bayesian Optimisation and Joint Inversion in Geosciences #2690

Closed
40 tasks done
whedon opened this issue Sep 22, 2020 · 70 comments
Assignees
Labels
accepted published Papers published in JOSS Python recommend-accept Papers recommended for acceptance in JOSS. review TeX

Comments

@whedon
Copy link

whedon commented Sep 22, 2020

Submitting author: @sebhaan (Sebastian Haan)
Repository: https://github.com/sebhaan/geobo
Version: v1.0.0
Editor: @hugoledoux
Reviewers: @npetra, @sgkang
Archive: 10.5281/zenodo.4451474

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/aeaacf1d3c09ebd193dba1b3c67e0312"><img src="https://joss.theoj.org/papers/aeaacf1d3c09ebd193dba1b3c67e0312/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/aeaacf1d3c09ebd193dba1b3c67e0312/status.svg)](https://joss.theoj.org/papers/aeaacf1d3c09ebd193dba1b3c67e0312)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@npetra and @sgkang, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @hugoledoux know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Review checklist for @sgkang

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@sebhaan) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @npetra

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@sebhaan) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?
@whedon
Copy link
Author

whedon commented Sep 22, 2020

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @npetra it looks like you're currently assigned to review this paper 🎉.

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

⭐ Important ⭐

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Sep 22, 2020

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@whedon
Copy link
Author

whedon commented Sep 22, 2020

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.21105/joss.00940 is OK

MISSING DOIs

- 10.5194/gmd-12-2941-2019 may be a valid DOI for title: Efficiency and robustness in Monte Carlo sampling for 3-D geophysical inversions with Obsidian v0. 1.2: setting up for success
- 10.5194/gmd-12-1-2019 may be a valid DOI for title: GemPy 1.0: open-source stochastic geological modeling and inversion

INVALID DOIs

- None

@hugoledoux
Copy link

@whedon add @sgkang as reviewer

@whedon whedon assigned hugoledoux and unassigned hugoledoux Oct 6, 2020
@whedon
Copy link
Author

whedon commented Oct 6, 2020

OK, @sgkang is now a reviewer

@sgkang
Copy link

sgkang commented Oct 16, 2020

@whedon: It seems the invitation that you have sent has expired. Can you invite again.
I am just starting this review.

@whedon
Copy link
Author

whedon commented Oct 16, 2020

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands

@sgkang
Copy link

sgkang commented Oct 16, 2020

@whedon add @sgkang as a reviewer

@whedon
Copy link
Author

whedon commented Oct 16, 2020

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands

@sgkang
Copy link

sgkang commented Oct 16, 2020

Hi @hugoledoux, can you invite me again? I am just starting the review, and the invitation link has expired.

@danielskatz
Copy link

@whedon re-invite @sgkang as reviewer

@whedon
Copy link
Author

whedon commented Oct 16, 2020

OK, the reviewer has been re-invited.

@sgkang please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

@npetra
Copy link

npetra commented Oct 27, 2020

Hi @hugoledoux, can you please resend the invite to me as well? Thanks! Plan to submit my initial review today or tomorrow. Thanks!

@npetra
Copy link

npetra commented Oct 28, 2020

Hi @sebhaan (& co),

GeoBO seems to be a useful and great software package for the community. Thanks for working on it and for sharing it with the community.

Here are my comments:

Functionality

  • The package is successfully installed by “python setup.py install”. However, it seems that the package is not registered in PyPI, and cannot be installed through ‘pip3’.

  • Is the software extensible to other physical problems (different linear forward models, e.g., elasticity models)? If it is, how intrusive such extension will be.

  • Is it possible to use a user-defined prior function?

  • What do the colors represent in the result figures, for example, in the figure ‘newdrill_proposals.png’?

  • It would be great to provide a clear explanation of the generated files and of the setting file (the authors provide some comments for input variables in the setting file, but it may not be enough to fully understand what the variables mean. For example, it is hard for me to get what the constants (c_G, c_SI_TO_MILLIAGALS, …) in the setting file mean).

Software/Codes

  • The copyright information is not complete and consistent throughout the code. There seems to be one single author for GeoBO. Perhaps make this clear in the Authors list/discussion.

Documentation

  • It would be great if the authors could also provide a Docker image. This would ease up the installation struggles users might have with the installation of the software and its dependencies.

  • There is no CONTRIBUTION file or section in README file for a guideline for third parties wishing to contribute to the software or to report issues or problems with the software.

Paper:

  • The author list is missing from the paper.

  • From the second setup it sounds like the authors also tackle optimal experimental design problems. If so, perhaps this should be clearly stated and a few references given.

@npetra npetra closed this as completed Oct 28, 2020
@npetra
Copy link

npetra commented Oct 28, 2020

Ups, sorry, it looks like I closed the issue by mistake. Trying to reopen it.

@npetra npetra reopened this Oct 28, 2020
@sebhaan
Copy link

sebhaan commented Oct 29, 2020

Hi @npetra,

thank you for your very helpful comments and taking your time to review this package.
I will have some more time to work on it next week and will come back to you soon with the details for each point.

@sgkang
Copy link

sgkang commented Nov 2, 2020

Dear @sebhaan et al.,

This is an interesting work simultaneously solving multi-objective optimization and joint inversion, and I am glad to have a chance to review this article. I got rough idea what authors are trying to accomplish with geobo from reading the article, but still it is not crystal clear to me. So, I was hoping to get better idea from examples. It successfully ran, and generated images and other outputs, but it was hard for me to decipher what they mean. I would strongly recommend having tutorial notebooks, or better form of documentation (e.g., sphinx) to ease the process of using geobo. I would love to read that documentation, and further play with what geobo could provide.

My detailed comments are following:

Functionality

  • Pypi version did not work, so I had to clone and run python setup.py install
  • There are test examples, but no tests. I was not sure how to quantify the performance of the code base. For instance, for geophysical simulation problem, I would expect to have comparison with analytic solution.

Documentation

  • I was not sure "the statement of need" is clear. For instance, why do I need to use geobo rather than using conventional mag. and gravity inversion methods (e.g. Li and Oldenburg, 1998)?
  • There is an example, and it runs. But, I was not sure that I could understand what the output means. There should be some description about the outputs.
  • I thought documentation is sparse. For the current form, I am not sure that I can run other examples than provided in the repository.
  • I have not found automated tests (e.g., travis-ci), and I recommend authors to generate test cases (not examples, but testing performance of the codes)
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • I was not sure what are the problems (or applications) that authors are having in mind. I felt it is designed to solve mining situations. Also, I was not sure who are the target audiences. So, these should be clarified.
  • Comparison with other software was missing. If the goal is imaging the distribution of physical properties, I recommend suggesting a typical 3D potential field inversion method (e.g. Li and Oldenburg, 1998).
  • Thought interesting that authors solves an optimal survey design problem. But, I was a bit lost how that problem is connected with joint inversion; it seems the connections are sensor data & fusion and world model with uncertainty (from Figure 1), but I was not sure what they mean.

@hugoledoux
Copy link

thanks @npetra and @sgkang for your prompt and thorough reviews.

Now we wait for @sebhaan to update the submission based on this, and we have one more iteration.

@sebhaan
Copy link

sebhaan commented Nov 15, 2020

Hi @npetra ,

thanks for your helpful review. I've updated the code and documentation accordingly. Please see below inline for the detailed reply

Functionality

  • The package is successfully installed by “python setup.py install”. However, it seems that the package is not registered in PyPI, and cannot be installed through ‘pip3’.

Re: It is now registered on PyPi (https://pypi.org/project/geobo/0.1.0/) and can be installed with pip.

  • Is the software extensible to other physical problems (different linear forward models, e.g., elasticity models)? If it is, how intrusive such extension will be.

Re: Yes, as long as the forward model is a linear or quasi-linear system (i.e. the relationship problem between a physical system phi and the observed sensor y can be reduced to y = G times phi, where G is the forward model operator or matrix). I've added a new subsection "Custom Linear Forward Models" to the README file, which should help to add any new model to the module sensormodel.py. The easiest approach would be to keep the prism model matrix structure, but in principle any function that returns a forward model matrix could be included. I'm not very familiar with elasticity models, so I don' have a answer for that specific problem.

  • Is it possible to use a user-defined prior function?

Re: Yes, it is possible to select from a range of kernel options as Gaussian Process prior functions (e.g. exponential squared or Matern kernel). The module kernels.py includes a range of kernel options, but in principle any other custom kernel can be added here. However, it is recommended to use the default sparse kernel to handle the computational problem O(N^3) of inverting a large covariance matrix. Future updates may include other solutions to scale to much larger cubes.
I've added some more detail about the prior functions in a new subsection "Gaussian Process Kernels".

  • What do the colors represent in the result figures, for example, in the figure ‘newdrill_proposals.png’?

Re: Added in README a description of result files in a new subsection "Results and Output Files". This also provides a more detailed explanation of the generated files (and colors).

  • It would be great to provide a clear explanation of the generated files and of the setting file (the authors provide some comments for input variables in the setting file, but it may not be enough to fully understand what the variables mean. For example, it is hard for me to get what the constants (c_G, c_SI_TO_MILLIAGALS, …) in the setting file mean).

Re: I've added more detailed description of some of the settings (Choices of GP kernel, Acquisition function) in the README under Options and Customizations. Also expanded description in settings file on the constants.

Software/Codes

  • The copyright information is not complete and consistent throughout the code. There seems to be one single author for GeoBO. Perhaps make this clear in the Authors list/discussion.

Re: Code has been updated with consistent copyright, license and author information. I confirm it is a one author paper.

Documentation

  • It would be great if the authors could also provide a Docker image. This would ease up the installation struggles users might have with the installation of the software and its dependencies.

Re: The package is now registered on PyPI and can be easily installed with pip3, which includes automatically all the dependencies. Thus Docker seems at the current stage not necessary.

  • There is no CONTRIBUTION file or section in README file for a guideline for third parties wishing to contribute to the software or to report issues or problems with the software.

Re: Added file CONTRIBUTION.md with instructions.

Paper:

  • The author list is missing from the paper.

Re: There is only one author, which is in paper.md and the article proof pdf (previously paper.pdf did omit the author due to the custom meta description in paper.md file.)

  • From the second setup it sounds like the authors also tackle optimal experimental design problems. If so, perhaps this should be clearly stated and a few references given.

Re: The second step is the actual Bayesian Optimisation (BO), which is now more clearly stated in the paper. Added also some more BO references to the paper.

@kthyng
Copy link

kthyng commented Jan 22, 2021

Hi @sebhaan I edited your paper. See if you agree with the changes in the PR and merge if so.

Also, it looks like at least some words that should be capitalized in your references are not capitalized. For example, Bayesian in Brochu et al and Gaussian in Melkumyan et al. Please go through all references in detail to fix these. You can add {} around words or characters to preserve capitalization.

@sebhaan
Copy link

sebhaan commented Jan 28, 2021

Hi @kthyng
thanks for fixing the typos. The changes are now merged with master. I've also checked and corrected the capitalization in the references and added the preservation of the original title capitalization by using {}. The words "Bayesian" and "Gaussian" are capitalized as eponyms. Thanks again for cross-checking.

@sebhaan
Copy link

sebhaan commented Jan 28, 2021

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jan 28, 2021

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@sebhaan
Copy link

sebhaan commented Jan 28, 2021

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jan 28, 2021

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@sebhaan
Copy link

sebhaan commented Jan 28, 2021

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Jan 28, 2021

I'm sorry @sebhaan, I'm afraid I can't do that. That's something only editor-in-chiefs are allowed to do.

@danielskatz
Copy link

@whedon accept

@whedon
Copy link
Author

whedon commented Jan 28, 2021

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Jan 28, 2021

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#2057

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2057, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

@danielskatz
Copy link

@sebhaan - in the .bib file, please spell out IJCAI. Once this is done, we can publish

@sebhaan
Copy link

sebhaan commented Jan 28, 2021

@danielskatz - .bib file updated. IJCAI replaced with International Joint Conference on Artificial Intelligence

@danielskatz
Copy link

@whedon accept

@whedon
Copy link
Author

whedon commented Jan 28, 2021

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Jan 28, 2021

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#2060

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2060, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Jan 28, 2021

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1190/geo2019-0460.1 is OK
- 10.21105/joss.00940 is OK
- 10.5194/gmd-12-2941-2019 is OK
- 10.1190/1.1581067 is OK
- 10.5194/gmd-12-1-2019 is OK
- 10.1111/j.1365-246X.2010.04856.x is OK
- 10.1111/j.1365-246x.1993.tb01452.x is OK

MISSING DOIs

- None

INVALID DOIs

- None

@danielskatz
Copy link

@whedon accept deposit=true

@whedon whedon added accepted published Papers published in JOSS labels Jan 28, 2021
@whedon
Copy link
Author

whedon commented Jan 28, 2021

Doing it live! Attempting automated processing of paper acceptance...

@whedon
Copy link
Author

whedon commented Jan 28, 2021

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

@whedon
Copy link
Author

whedon commented Jan 28, 2021

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.02690 joss-papers#2061
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02690
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

@danielskatz
Copy link

Congratulations to @sebhaan (Sebastian Haan)!!

And thanks to @npetra and @sgkang for reviewing, and @hugoledoux for editing!

@whedon
Copy link
Author

whedon commented Jan 28, 2021

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.02690/status.svg)](https://doi.org/10.21105/joss.02690)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.02690">
  <img src="https://joss.theoj.org/papers/10.21105/joss.02690/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.02690/status.svg
   :target: https://doi.org/10.21105/joss.02690

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted published Papers published in JOSS Python recommend-accept Papers recommended for acceptance in JOSS. review TeX
Projects
None yet
Development

No branches or pull requests

7 participants