Closed whedon closed 3 years ago
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 :tada:.
:warning: JOSS reduced service mode :warning:
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.
:star: Important :star:
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:
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
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
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
@whedon add @sgkang as reviewer
OK, @sgkang is now a reviewer
@whedon: It seems the invitation that you have sent has expired. Can you invite again. I am just starting this review.
I'm sorry human, I don't understand that. You can see what commands I support by typing:
@whedon commands
@whedon add @sgkang as a reviewer
I'm sorry human, I don't understand that. You can see what commands I support by typing:
@whedon commands
Hi @hugoledoux, can you invite me again? I am just starting the review, and the invitation link has expired.
@whedon re-invite @sgkang as reviewer
OK, the reviewer has been re-invited.
@sgkang please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations
Hi @hugoledoux, can you please resend the invite to me as well? Thanks! Plan to submit my initial review today or tomorrow. Thanks!
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
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.
Ups, sorry, it looks like I closed the issue by mistake. Trying to reopen it.
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.
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:
python setup.py install
geobo
rather than using conventional mag. and gravity inversion methods (e.g. Li and Oldenburg, 1998)?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.
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.
- [x] 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.
Dear @sgkang
thank you for taking your time to review this package and your helpful comments. I've updated the code, paper, and documentation accordingly, but still have to add some more automated tests. Please see below inline for the detailed reply.
Dear @sebhaan et al.,
This is an interesting work, and I am glad to have a chance to review this article. When reading this article, I got a rough idea what authors are trying to accomplish with
geobo
, but still it was not clear to me. So, I was hoping to get better idea from examples. But, it ended up generating images and other outputs, which were hard for me to decipher what they mean. I would recommend having tutorial notebooks, or better form of documentation (e.g., sphinx).My detailed comments are following:
Functionality
- [x] Pypi version did not work, so I had to clone and run
python setup.py install
Re: It is now registered on PyPi (https://pypi.org/project/geobo/0.1.0/) and can be installed with pip.
- [x] 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.
Re: Will add some more automated test (likely with pytest). I presume the analytic solution refers to the forward model testing. In principle one could include a test of the forward model analytically for one voxel at a given distance from sensor on surface. GeoBO has been tested using the following approach by "closing the circle test" between forward modeling and inversion. Assuming the forward model is correct (e.g. via one voxel test), the simulation creates first synthetic 3D subsurface properties, then computes the corresponding sensor measurements on the surface using the deterministic forward model, and then can finally test if the inversion can reconstruct the synthetic 3D subsurface properties again.
Documentation
- [x] 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)?
Re: As described in the updated paper there are two main advantages in comparison to traditional inversion methods: First,one key advantage of this method is to solve simultaneously multiple sensor measurements by taking into account the correlations between distinct geological aspect, e.g., density and magnetic susceptibility. This joint optimization has the advantage to find the best possible solution over the complete parameter space. Another practical advantage of this probabilistic approach is that predictions are described by posterior distributions for each location (voxel or cube cell), which quantify the uncertainty in the predictions and their credible intervals. Most other work in the field of joint inversion (e.g. Li and Oldenburg, 1998) only report point estimates of the quantities of interest. This ability to propagate consistently uncertainties from input to output space under the Bayesian inversion formalism is an essential requirement for the subsequent Bayesian optimisation to search for new optimal measurement locations (since the acquisition function needs the predicted uncertainty cube as input).
- [x] 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.
Re: Added to README file and paper an additional section (Results and Output Files) to describe the output.
- [x] I thought documentation is sparse. For the current form, I am not sure that I can run other examples than provided in the repository.
Re: Added a couple of new sections in the README file to describe better the user and customization options such as "Custom Linear Forward Models" , which should help to add any new forward model to the module sensormodel.py. Also more details about Gaussian Process Kernel Functions and Bayesian Optimisation Options have been added to README. For more mathematical details, please see the paper "Multi-Objective Bayesian Optimisation and Joint Inversion for Active Sensor Fusion" (https://arxiv.org/abs/2010.05386)
- [x] 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)
Re: Yes, it currently doesn't has any automated test via pytest or travis given the complexity of user options, except for running the test examples. On the current To-Do list is adding automated testing of these and other critical functions to the code. Also testing the performance of the code via code profilers can be added.
- [x] 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
Re: Added file CONTRIBUTION.md with instructions
Software paper
- [x] 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.
Re: Added more description to paper. While the current implementation generates 3D geophysical properties based on gravity and magnetic inversion and searching for optimal new drillcore measurements, in principle the same model can be applied to a wider range of sensor fusion problems and allocation problems. For instance: What is the optimal mixture and placement of different sensors? Where to sample if the cost function is incomplete or if there are large uncertainties in future total budget allocations? How to position sensor grids over time if the model state is dynamic or has a moving target? Moreover GeoBO is a data-driven approach and uses Gaussian Process method for solving the inversion problem. The advantages of the Gaussian Process method is that it is tractable, generates posterior distributions, can predict at any resolution or scale, and can be optimised via marginal log likelihood. An alternative to Gaussian Process method could be using sampling methods (e.g. via MCMC or variational inference) but they are computationally very expensive given their need to sample over an almost infinite range of possible models, and are therefore in practice typically constrained by certain assumptions about the underlying geological formation and prior parameterisation.
- [x] 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).
Re: See comment above (the reply on "the statement of need"). Li and Oldenburg is not a joint inversion and does not generate posterior distributions (uncertainty cubes).
- [x] 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.
Re: Bayesian Optimisation (BO) does not necessarily need a joint inversion problem but requires that the inversion method (or any 3D surrogate world modeling) - no matter whether it is a joint or simple - generates cubes with location dependent uncertainty predictions (heteroscedastic noise). The objective function in BO requires the information about the uncertainty to find new locations that increase information gain (reduce uncertainty). The Gaussian Process inversion method produces basically a probabilistic surrogate model of the 3D world properties. Note that BO is not to the same as the "Design of Experiment" method that aims to search for an effective initial parameter (experiment variables) configuration typically BEFORE any measurement is taken (and before any model evaluation is possible). BO aims to improve a model typically AFTER some initial measurements are taken, which allows us to build a surrogate world model.
@sebhaan Thanks very much for addressing all my comments. Please give me 1-2 days to go over the changes and try the new install.
@sebhaan The new install seems to work and all my comments have been addressed. This paper is good to go from my perspective.
@sebhaan The new install seems to work and all my comments have been addressed. This paper is good to go from my perspective.
thanks @npetra , could you check the list items above please if all is satisfactory?
👋 @sgkang, please update us on how your review is going, the authors have addressed your comments and we're waiting for your comments on those.
@hugoledoux and @sebhaan , sounds great, and thanks for heads up. I'll finalize the review by the end of this week.
Dear @sebhaan,
My comments are addressed, and the paper is looking good to me. Look forward to this paper is out the door!
Below are my suggests that I thought while I was reviewing your paper and code base.
Cockett, R., Kang, S., Heagy, L. J., Pidlisecky, A., & Oldenburg, D. W. (2015). SimPEG: An open source framework for simulation and gradient based parameter estimation in geophysical applications. Computers & Geosciences, 85, 142–154. https://doi.org/http://dx.doi.org/10.1016/j.cageo.2015.09.015"""
Rücker, C., Günther, T., & Wagner, F. M. (2017). {pyGIMLi}: An open-source library for modelling and inversion in geophysics. Computers and Geosciences, 109, 106–123. https://doi.org/10.1016/j.cageo.2017.07.011
Generalization of the code (?). Had a brief look at Inversion
class, and it is tied to magnetic and gravity methods, so making the inversion class bit more general seems a reasonable step towards.
Testing of the code. Current form of testing, which runs a synthetic example and check the results seems okay at the moment. But, for a long run, I would recommend implementing more rigorous tests, more than testing with the synthetic example (e.g. expected property of your optimization scheme...). Further use the unittest
, and automate the testing procedure such that you can readily deploy of continuous integration platform like travis.
Hope this helps,
All the best,
Seogi.
Hi @sgkang,
thanks so much for your suggestions, on the list now for the next version. The suggested inversion packages are very interesting as well and could potentially be included if they can provide predictive model uncertainties for the BO function.
Thanks again for the review, Seb
@whedon generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@whedon commands
Here are some things you can ask me to do:
# List all of Whedon's capabilities
@whedon commands
# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer
# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer
# Re-invite a reviewer (if they can't update checklists)
@whedon re-invite @username as reviewer
# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
# Change editorial assignment
@whedon assign @username as editor
# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive
# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version
# Open the review issue
@whedon start review
EDITORIAL TASKS
# All commands can be run on a non-default branch, to do this pass a custom
# branch name by following the command with `from branch custom-branch-name`.
# For example:
# Compile the paper
@whedon generate pdf
# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name
# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks
# Ask Whedon to do a dry run of accepting the paper and depositing with Crossref
@whedon accept
# Ask Whedon to check the references for missing DOIs
@whedon check references
# Ask Whedon to check repository statistics for the submitted software
@whedon check repository
EiC TASKS
# Invite an editor to edit a submission (sending them an email)
@whedon invite @editor as editor
# Reject a paper
@whedon reject
# Withdraw a paper
@whedon withdraw
# Ask Whedon to actually accept the paper and deposit with Crossref
@whedon accept deposit=true
@whedon check references
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.21105/joss.00940 is OK
MISSING DOIs
- 10.1190/geo2019-0460.1 may be a valid DOI for title: Multi-Objective Bayesian Optimisation and Joint Inversion for Active Sensor Fusion
- 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.1190/1.1581067 may be a valid DOI for title: A versatile algorithm for joint 3D inversion of gravity and magnetic data
- 10.5194/gmd-12-1-2019 may be a valid DOI for title: GemPy 1.0: open-source stochastic geological modeling and inversion
- 10.1190/1.3513658 may be a valid DOI for title: A framework for 3-D joint inversion of MT, gravity and seismic refraction data
- 10.1111/j.1365-246x.1993.tb01452.x may be a valid DOI for title: 3-D joint inversion of magnetic and gravimetric data with a priori information
INVALID DOIs
- None
ok @sebhaan it seems that the reviewers are now happy with the changes you made, and we can move towards the final acceptance.
I checked you PDF and above you can see that many DOI are missing, could you please add them and run again here @whedon check references
please?
Also, the way you added references was sometimes wrong, so I fixed them (plus 2-3 minor cosmetic things) in that PR: https://github.com/sebhaan/geobo/pull/1
@whedon check references
@whedon generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
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
Great.
At this point could you:
I can then move forward with accepting the submission.
Hi @hugoledoux,
thanks for the editorial work. I've finished the suggested points, see below inline.
Great.
At this point could you:
- [ ] Make a tagged release of your software, and list the version tag of the archived version here. Re: Release tag: v1.0.0
- [ ] Archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository) Re: Done in Zenodo
- [ ] Check the archival deposit (e.g., in Zenodo) has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it). You may also add the authors' ORCID. Re: Checked, see linke here https://zenodo.org/record/4451474
- [ ] Please list the DOI of the archived version here. Re: Zenodo DOI: 10.5281/zenodo.4451474
I can then move forward with accepting the submission.
@whedon set 10.5281/zenodo.4451474 as archive
OK. 10.5281/zenodo.4451474 is the archive.
@whedon set v1.0.0 as version
OK. v1.0.0 is the version.
@whedon accept
Attempting dry run of processing paper acceptance...
:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2042
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/2042, then you can now move forward with accepting the submission by compiling again with the flag deposit=true
e.g.
@whedon accept deposit=true
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.
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.
@whedon generate pdf
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
:warning: JOSS reduced service mode :warning:
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 badge code:
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:
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
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @npetra
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper