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

Paper revised version #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Paper revised version #101

wants to merge 1 commit into from

Conversation

daavid00
Copy link
Collaborator

@daavid00 daavid00 commented Dec 3, 2024

This PR addresses the paper review in https://github.com/OPM/pyopmspe11/pull/84/files and #89.

@totto82 and I we thank both reviewers @gassmoeller and @MatthewFlamm for their valuable comments.

We believe we have addressed all of them, which has significantly improved the quality of the paper, many thanks again.

Please let us know if this PR can be merged, or if additional changes are requiered.

Copy link
Contributor

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

Yup, much better. I have some suggestions for the wording and you should check your paper one more time for typos. However, afterwards this is ready from my point of view.

paper/paper.md Outdated
Project via configuration files. `pyopmspe11` relies on the OPM Flow numerical simulator [@Rassmussen:2021], where the


Based on the acquired knowledge by developing the aforementioned tools, as well as from using/contributing to other open-source projects, then we have developed and made open the `pyopmspe11` tool which facilitates reproducible solutions to the SPE11 benchmark, which focus on GCS at different scales [@Nordbotten:2024].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Based on the acquired knowledge by developing the aforementioned tools, as well as from using/contributing to other open-source projects, then we have developed and made open the `pyopmspe11` tool which facilitates reproducible solutions to the SPE11 benchmark, which focus on GCS at different scales [@Nordbotten:2024].
Based on the acquired knowledge by developing the aforementioned tools, as well as from using/contributing to other open-source projects, we have developed and made open the `pyopmspe11` tool. `pyopmspe11` facilitates reproducible solutions to the SPE11 benchmark, which focuses on GCS at different scales [@Nordbotten:2024].

paper/paper.md Outdated


Based on the acquired knowledge by developing the aforementioned tools, as well as from using/contributing to other open-source projects, then we have developed and made open the `pyopmspe11` tool which facilitates reproducible solutions to the SPE11 benchmark, which focus on GCS at different scales [@Nordbotten:2024].
A previous benchmark study for GCS can be found in @Class:2009. One key differece of the SPE11 benchmark from the previous one is that no grids were given in the description, i.e., one of the main task for the participants was to create suitable grids (e.g., structured grids such as Cartesian or unscrtuctured grids such as corner-point grids). The participants were encouraged to share data (e.g., input decks, code, submitted results), with the opportunity to store the data for open access. This is where developing tools that made all steps reprodubible (i.e., preprocessing and postprocessing) become handy, and for this benchmark study, one available tool is `pyopmspe11`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A previous benchmark study for GCS can be found in @Class:2009. One key differece of the SPE11 benchmark from the previous one is that no grids were given in the description, i.e., one of the main task for the participants was to create suitable grids (e.g., structured grids such as Cartesian or unscrtuctured grids such as corner-point grids). The participants were encouraged to share data (e.g., input decks, code, submitted results), with the opportunity to store the data for open access. This is where developing tools that made all steps reprodubible (i.e., preprocessing and postprocessing) become handy, and for this benchmark study, one available tool is `pyopmspe11`.
A previous benchmark study for GCS can be found in @Class:2009. One key differece of the SPE11 benchmark from the previous one is that no grids were given in the description, i.e., one of the main task for the participants was to create suitable grids (e.g., structured grids such as Cartesian or unstructured grids such as corner-point grids). The participants were encouraged to share data (e.g., input decks, code, submitted results), with the opportunity to store the data for open access. This is where developing tools that made all steps reproducible (i.e., preprocessing and postprocessing) become handy, and for this benchmark study, one available tool is `pyopmspe11`.

paper/paper.md Outdated
compatible not only with OPM Flow but also with other simulators. Additionally, `pyopmspe11` supports different resolutions,
having been tested to generate approximately 160 million cells. In the context of data postprocessing, `pyopmspe11` not only
including Cartesian, tensor, and corner-point grids. These grids adhere to the standard industry format (i.e., Eclipse grid format), making them
compatible not only with OPM Flow but also with other simulators. Here, we mention two existing widely-use visualization/postrocessing software for OPM Flow: [ParaView](https://www.paraview.org) and [ResInsight](https://resinsight.org). While these tools are very useful, to the authors knolwedge, there is no existing functionality in these tools to handle all the necessary postporcessing to generate all data reporting as required in the SPE11 benchmark study. For example, to write the csv maps of the quantities from a simulation grid to a given reporting grid, where both grids do not overlap, or to compute the convective mixing as defined in Eq. (17) in [@Nordbotten:2024].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
compatible not only with OPM Flow but also with other simulators. Here, we mention two existing widely-use visualization/postrocessing software for OPM Flow: [ParaView](https://www.paraview.org) and [ResInsight](https://resinsight.org). While these tools are very useful, to the authors knolwedge, there is no existing functionality in these tools to handle all the necessary postporcessing to generate all data reporting as required in the SPE11 benchmark study. For example, to write the csv maps of the quantities from a simulation grid to a given reporting grid, where both grids do not overlap, or to compute the convective mixing as defined in Eq. (17) in [@Nordbotten:2024].
compatible not only with OPM Flow but also with other simulators. Here, we mention two existing widely-use visualization/postprocessing software for OPM Flow: [ParaView](https://www.paraview.org) and [ResInsight](https://resinsight.org). While these tools are very useful, to the authors knowledge, there is no existing functionality in these tools to generate all postprocessed data as required in the SPE11 benchmark study. For example, to write the csv maps of the quantities from a simulation grid to a given reporting grid, where both grids do not overlap, or to compute the convective mixing as defined in Eq. (17) in [@Nordbotten:2024].

Not sure the last sentence is quite correct (you can do a lot of interpolation to other specified grids and/or complex calculations (even embedded python functions) inside Paraview. However, I of course see the benefit of having all that already readily created in a specialized tool, so feel free to keep the sentence.

Choose a reason for hiding this comment

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

Bias disclosure: I am a maintainer of PyVista.

I am not well versed in all of the requirements for the SPE11 benchmark postprocessing. However, the example given of writing a csv file corresponding to points on a reporting grid sampled from a simulation grid can be done in some of the tools listed here. For example from PyVista referenced elsewhere in the paper, there is a similar example here https://docs.pyvista.org/examples/01-filter/resample. And, then writing to csv can be done given the numpy-like nature of data access in PyVista. So I think the tools listed can likely generate the post-proccessed data, particularly when multiple tools are used together, but to get it right requires a lot of development. There is a lot of value in a standard post-processing tool for a benchmark like this. Otherwise, different researchers might get differing results solely due to post-processing artifacts.

So I recommend rewording these sentences to focus on the advantages of pyopmsep11 here instead of saying that these tools cannot generate the data.

Choose a reason for hiding this comment

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

This is also not a full sentence:

For example, to write the csv maps of the quantities from a simulation grid to a given reporting grid, where both grids do not overlap, or to compute the convective mixing as defined in Eq. (17) in [@nordbotten:2024].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @MatthewFlamm for your comments. In the text we wrote "existing functionality" to mean that to our knowledge there is no built-in options in the GUIs of ResInsight or Paraview to handle all post-process as required from the benchmark, specially mapping extensive quantities for non-overlapping 3D grids with different data formats (e.g., corner-point grids with Eclipse format to Cartesian grids). We agree with you that this functionality can be added to these tools after development, and we did not intend to mean that these tools cannot generate the data, sorry for that. We have rewrote the text, focusing on the advantages and benefits of having all steps (preprocessing, simulation, and postprocessing) integrated in one tool. We have also removed the last sentence as also suggested by @gassmoeller.

paper/paper.md Outdated
generates the necessary reporting data as specified by the benchmark, but it also produces .png figures for rapid inspection
of individual simulations and for making comparisons between different runs (e.g., to assess sensitivities). The postprocessing
methods efficiently map non-overlapping cell values (both intensive and extensive quantities) between the simulation grid and
the reporting grid.
methods efficiently interpolates quantities over time and map non-overlapping cell values (both intensive and extensive quantities) between the simulation grid and the reporting grid. The Python package Scipy [@Virtanen:2020], specifically the interp1d Class, is used for the time interpolation. The Python package Shapely [@Gillies:2024], speficically the Polygon Class, is the base for the developed methods in `pyopmspe11` to handle the mapping from the simulation grid to the reporting grid.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
methods efficiently interpolates quantities over time and map non-overlapping cell values (both intensive and extensive quantities) between the simulation grid and the reporting grid. The Python package Scipy [@Virtanen:2020], specifically the interp1d Class, is used for the time interpolation. The Python package Shapely [@Gillies:2024], speficically the Polygon Class, is the base for the developed methods in `pyopmspe11` to handle the mapping from the simulation grid to the reporting grid.
methods efficiently interpolates quantities over time and map non-overlapping cell values (both intensive and extensive quantities) between the simulation grid and the reporting grid. The Python package Scipy [@Virtanen:2020], specifically the interp1d Class, is used for the time interpolation. The Python package Shapely [@Gillies:2024], specifically the Polygon Class, is the base for the developed methods in `pyopmspe11` to handle the mapping from the simulation grid to the reporting grid.

@daavid00
Copy link
Collaborator Author

Many thanks @gassmoeller for your comments and corrections. We will wait for the comments from @MatthewFlamm before merging this 🙂.

@MatthewFlamm
Copy link

I have added my comments inside this PR.

@MatthewFlamm
Copy link

You can close #89 when this is merged.

@MatthewFlamm
Copy link

I'm happy with the changes. Thanks!

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.

3 participants