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

Patch reset cavern sg on new stage start #25

Merged
merged 10 commits into from
Oct 1, 2024
Merged
27 changes: 18 additions & 9 deletions .github/workflows/build-wheels.yml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is pulling in build process changes and is not related to the bug fix

Original file line number Diff line number Diff line change
@@ -1,22 +1,31 @@
name: Build wheels (intermediate)

on:
push:
branches:
- "*patch*"
workflow_dispatch:

jobs:

wheels:
build_wheels:
name: Build distribution 📦 on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [windows-latest, macOS-13, macOS-latest, ubuntu-latest]
# macos-13 is an intel runner, macos-14 is apple silicon
os: [ubuntu-latest, windows-latest, macos-13, macos-14]

steps:
- uses: actions/checkout@v4
- name: Harden Runner
uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
with:
egress-policy: audit
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7

- name: Build wheels
uses: pypa/[email protected]
env:
CIBW_BUILD: cp39-* cp310-* cp311-* cp312-*
CIBW_SKIP: "*-win32 *-manylinux_i686 pp* *-musllinux*"
- uses: actions/upload-artifact@v4
uses: pypa/cibuildwheel@d4a2945fcc8d13f20a1b99d461b8e844d5fc6e23 # v2.21.1

- uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
with:
name: cibw-wheels-${{ matrix.os }}-${{ strategy.job-index }}
path: ./wheelhouse/*.whl
1 change: 1 addition & 0 deletions .gitignore
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not part of the bugfix, this change helps VSCode users not check in private workspace files

Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,4 @@ _local/
docs/apidocs/pymodule/**
docs/apidocs/clibrary/**
DEPLOY.bat
**/*.code-workspace
3 changes: 3 additions & 0 deletions README.md
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not part of the bugfix, this provides links to badges on pypi

Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ be okay with that :grin: )


#### PyPI wheels
[![pypi](https://img.shields.io/pypi/v/sansmic.svg?maxAge=3600)](https://pypi.org/project/sansmic/)
[![Downloads](https://static.pepy.tech/badge/sansmic)](https://pepy.tech/project/sansmic)

To install a pre-compiled version of sansmic, use the pip command

python -m pip install sansmic
Expand Down
19 changes: 12 additions & 7 deletions src/ext_modules/libsansmic/model.cpp
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Related to the bugfix. This moved the assignment to the outside of the for loop (which was unnecessary)

Unrelated to the bugfix, adds more explicit license details

Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,14 @@ int sansmic::Model::init_stage(void) {
// initialize variables associated with the cavern sg
for (int i = 1; i <= n_nodes; i++) {
f_disSav[i] = f_dis[i];

// FIXME: do we really want this next line?
if (runMode == Withdrawal && C_cav0 > 1.0) {
C_cav[i] = C_cav0;
}
}
// FIXME: do we really want this next line?
if (runMode == Withdrawal && C_cav0 > 1.0) {
std::fill(C_cav.begin(), C_cav.end(), C_cav0);
C_cav[n_nodes] = C_hat;
C_cavAve = C_cav0;
}

std::fill(V_injSigned.begin(), V_injSigned.end(), 0.0);
std::fill(x_incl.begin(), x_incl.end(), 0.0);
std::fill(V_saltRemove.begin(), V_saltRemove.end(), 0.0);
Expand Down Expand Up @@ -1673,8 +1675,11 @@ void sansmic::Model::close_outfiles(void) {
* @param sout the file to write to
*/
void sansmic::Model::write_header(ofstream &sout) {
sout << "# Generated by sansmic v" << VERSION_INFO << " under the BSD-3-Clause license." << endl;
sout << "# [sansmic (c) 2024 NTESS, https://github.com/SandiaLabs/sansmic/blob/main/LICENSE]" << endl;
sout << "# Generated by sansmic v" << VERSION_INFO
<< " under the BSD-3-Clause license." << endl;
sout << "# [sansmic (c) 2024 NTESS, "
"https://github.com/SandiaLabs/sansmic/blob/main/LICENSE]"
<< endl;
}

/**
Expand Down
13 changes: 2 additions & 11 deletions src/python/sansmic/io.py
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updates warnings and does not reset cavern sg on "repeat"; this returns sansmic to previous behavior.

Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,10 @@ def read_dat(str_or_buffer, *, ignore_errors=False) -> Scenario:
)
)
raise TypeError("Invalid data in DAT file: RESETGEO not 0")
if not first and not bool(1 - int(subsequent)):
if not first and isinstance(cavern_sg, (float, int)) and cavern_sg > 1.0:
logger.warning(
"The REPEAT option is supposed to turn off the cavern SG; initial cavern SG set to None."
"The REPEAT option was supposed to turn off the cavern SG; it did not do so. sansmic is currently mimicing SANSMIC behavior and resetting the cavern brine to {} sg in stage {}. \n\nIf this is not what is intended, please manually remove the 'set-cavern-sg' entry from stages after stage 1. This behavior will change in future releases.".format(cavern_sg, len(scenario.stages)+1)
)
cavern_sg = 0.0
stage.title = title
stage.simulation_mode = SimulationMode(int(mode))
stage.save_frequency = int(print_interval)
Expand Down Expand Up @@ -359,14 +358,6 @@ def write_scenario(scenario: Scenario, filename: str, *, redundant=False, format
):
del s["stop-condition"]
del s["stop-value"]
if scenario.stages[ct].set_initial_conditions is False and ct > 0:
s["set-cavern-sg"] = None
s["brine-interface-depth"] = None
s["set-initial-conditions"] = None
elif scenario.stages[ct].set_initial_conditions is None and ct > 0:
s["set-cavern-sg"] = None
if not s.get("brine-interface-depth", None) and redundant:
s["brine-interface-depth"] = 0
keys = [k for k in s.keys()]
for k in keys:
if (
Expand Down
43 changes: 18 additions & 25 deletions src/python/sansmic/model.py
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bug fix: removes the override-to-0 of the cavern SG on "REPEAT" stages that was in the Stage creation section.

Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,7 @@ class StageDefinition:
Automatically set to True for the first stage added to a model."""
set_cavern_sg: float = None
"""Set the initial specific gravity for all brine-filled cells of the cavern to
this value. If set_initial_conditions is False and this is not None (or 0) an error
will be raised upon scenario validation."""
this value."""
brine_interface_depth: float = None
"""Set the initial oil-brine interface or blanket level. By default None, which
will link to the previous stage (as will a value of 0)."""
Expand Down Expand Up @@ -369,7 +368,7 @@ def __post_init__(self, defaults=None):
for k, v in defaults.items():
k2 = k.strip().replace("-", "_").replace(" ", "_").replace(".", "_")
if k2 not in self.valid_default_keys:
logger.warning(
logger.warning( # pragma: no cover
"Ignoring non-defaultable or unknown setting {} = {}".format(
k, repr(v)
)
Expand Down Expand Up @@ -402,9 +401,9 @@ def __setattr__(self, name, value):
TypeError("stop_condition cannot be of type {}".format(type(value)))
elif name == "set_cavern_sg" and value is not None and value < 1.0:
value = None
elif name == "set_initial_conditions" and value is False:
self.set_cavern_sg = None
self.brine_interface_depth = None
# elif name == "set_initial_conditions" and value is False:
# self.set_cavern_sg = None
# self.brine_interface_depth = None
super().__setattr__(name, value)

@classmethod
Expand Down Expand Up @@ -486,22 +485,22 @@ def validate(self):

# Validate appropriate initial conditions settings
if self.set_initial_conditions and self.set_cavern_sg is None:
logger.warn(
logger.warning( # pragma: no cover
"Setting the initial conditions without setting cavern sg -- cavern sg will be set to fully saturated brine"
)
self.set_cavern_sg = 10.0
elif (
not self.set_initial_conditions
and self.set_cavern_sg is not None
and self.set_cavern_sg >= 1.0
):
# raise TypeError(
logger.warn(
"Setting the starting cavern sg requires set_initial_conditions to be set to True -- setting to 0.0"
)
self.set_cavern_sg = 0.0
# elif (
# not self.set_initial_conditions
# and self.set_cavern_sg is not None
# and self.set_cavern_sg >= 1.0
# ):
# # raise TypeError(
# logger.warning( # pragma: no cover
# "Setting the starting cavern sg requires set_initial_conditions to be set to True -- setting to 0.0"
# )
# self.set_cavern_sg = 0.0
elif not self.set_initial_conditions and self.brine_interface_depth:
logger.warn(
logger.warning( # pragma: no cover
"Make sure you meant to reset the interface level; use 0.0 or None to continue from the last stage."
)

Expand Down Expand Up @@ -698,13 +697,7 @@ def _to_cstage(self, defaults=None) -> _ext.CStage:
else self.inner_tbg_outside_diam / 2.0
)
stage.injection_fluid_sg = self.brine_injection_sg
stage.cavern_sg = (
0.0
if not self.set_initial_conditions
or self.set_cavern_sg is None
or self.set_cavern_sg < 1.0
else self.set_cavern_sg
)
stage.cavern_sg = self.set_cavern_sg if self.set_cavern_sg is not None else 0.0
stage.timestep = self.solver_timestep
stage.injection_duration = self.injection_duration
stage.fill_rate = (
Expand Down
2 changes: 1 addition & 1 deletion tests/test_io.py
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bug fix: verifies that value was not changed when specified as non-zero in a REPEAT section.

Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ def test_read_scenario_ordinary_reverse_multistage(self):
self.assertEqual(scenario.stages[2].outer_csg_inside_diam / 2, 8.925)
self.assertEqual(scenario.stages[2].outer_csg_outside_diam / 2, 9.375)
self.assertEqual(scenario.stages[2].brine_injection_sg, 1.03)
self.assertIsNone(scenario.stages[2].set_cavern_sg)
self.assertEqual(scenario.stages[2].set_cavern_sg, 1.2019)
self.assertEqual(scenario.stages[2].solver_timestep, 0.1)
self.assertEqual(scenario.stages[2].injection_duration, 72)
self.assertEqual(scenario.stages[2].product_injection_rate, 0.0)
Expand Down