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

Docs for opm common and embedded #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lisajulia
Copy link

These are the changes from OPM/opm-common#4018 and OPM/opm-simulators#5349

@lisajulia
Copy link
Author

@hakonhagland can you have a look? Thanks!

@lisajulia lisajulia force-pushed the docs-for-opm-common-and-embedded branch from 7cfc36a to 80c0eb0 Compare June 17, 2024 16:01
@lisajulia lisajulia force-pushed the docs-for-opm-common-and-embedded branch from 80c0eb0 to 69c9c34 Compare June 17, 2024 16:03
@hakonhagland
Copy link
Collaborator

@lisajulia Great, I will have a look later today 😄

set(DOCSTRINGS_GENERATED_HEADER_PATH "${PROJECT_BINARY_DIR}/${TEMP_GEN_DIR}/${DOCSTRINGS_GENERATED_HPP}")
set(PYBLACKOIL_DOCSTRINGS_FILE "${PROJECT_SOURCE_DIR}/docs/docstrings_simulators.json")
set(PYBLACKOIL_DOCSTRINGS_GENERATED_HPP "PyBlackOilSimulatorDoc.hpp")
set(PYBLACKOIL_DOCSTRINGS_GENERATED_HEADER_PATH "${PROJECT_BINARY_DIR}/${TEMP_GEN_DIR}/${PYBLACKOIL_DOCSTRINGS_GENERATED_HPP}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this file, only PYBLACKOIL_DOCSTRINGS_GENERATED_HEADER_PATH and COMMON_DOCSTRINGS_GENERATED_HEADER_PATH needs to be exported to the global scope, right? I like to export only necessary variables, I think it helps with maintenance in the long run. Could we make this a function like in e.g. OpmPythonSetupInstall.cmake ? I realize this was something I forgot to do in the original version of this file..


In order to enable the PYACTION keyword:

1. OPM flow must be compiled with the cmake switches -DOPM ENABLE EMBEDDED PYTHON=ON and -DOPM ENABLE PYTHON=ON, the default is to build with these switches set to OFF.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The plan is to remove the -DOPM_ENABLE_PYTHON flag from opm-common and only keep the -DOPM ENABLE EMBEDDED PYTHON flag. When that is done, we should update this sentence.

docs/docs/embedded.rst Show resolved Hide resolved
docs/docs/embedded.rst Show resolved Hide resolved
docs/docs/embedded.rst Show resolved Hide resolved
docs/docstrings_common.json Show resolved Hide resolved
docs/docstrings_common.json Show resolved Hide resolved
docs/docstrings_simulators.json Show resolved Hide resolved
docs/docstrings_simulators.json Show resolved Hide resolved
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.

2 participants