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

Remove control type filed #9901

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions docs/everest/minimal_example.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ everest_config.yml::
controls:
-
name: point
type: generic_control
min: -1.0
max: 1.0
initial_guess: 0
Expand All @@ -57,7 +56,6 @@ everest_config.yml::
controls:
-
name: point
type: generic_control
initial_guess: 0
perturbation_magnitude : 0.001
variables:
Expand Down Expand Up @@ -216,7 +214,6 @@ everest_config.yml::
controls:
-
name: point
type: generic_control
min: -1.0
max: 1.0
initial_guess: 0
Expand Down
13 changes: 3 additions & 10 deletions src/everest/config/control_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,9 @@ class ControlConfig(BaseModel):
name: Annotated[str, AfterValidator(no_dots_in_string)] = Field(
description="Control name"
)
type: Literal["well_control", "generic_control"] = Field(
description="""
Only two allowed control types are accepted

* **well_control**: Standard built-in Everest control type designed for field\
optimization

* **generic_control**: Enables the user to define controls types to be employed for\
customized optimization jobs.
"""
type: str | None = Field(
default=None,
description="""Type has been deprecated and can be removed from the config""",
)
variables: Annotated[
ControlVariable,
Expand Down
15 changes: 9 additions & 6 deletions src/everest/config/everest_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,15 +452,18 @@ def validate_variable_name_match_well_name(self) -> Self: # pylint: disable=E02
wells = self.wells
if controls is None or wells is None:
return self
well_names = [w.name for w in wells]
well_names = {w.name for w in wells}
if not well_names:
return self
for c in controls:
if c.type == "generic_control":
continue
for v in c.variables:
if v.name not in well_names:
raise ValueError("Variable name does not match any well name")
variable_names = {v.name for v in c.variables}
if len(variable_names.intersection(well_names)) not in {
0,
len(variable_names),
}:
raise ValueError(
"Variable names should either all or none match well names"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

So this case would not be allowed anymore: A control group where most variable names are arbitrary, but one or more happen to be well names?

If so, it seems an arbitrary limitation, somebody may have a case that breaks it.

Copy link
Contributor Author

@DanSava DanSava Jan 29, 2025

Choose a reason for hiding this comment

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

Yes, that will no longer be allowed.

We could remove this arbitrary limitation but once the control type filed is removed we can no longer enforce any constraint on the well name and variable names. So as far as I can see there we have the following options:

  1. We keep the control type.
  2. We remove the control type and this validation
  3. We add this arbitrary limitation that might break some cases where there are well_controls and generic_controls that happen to have some of the variable names match the well names.

There could be some other options I am not considering at the moment, but I am open to suggestions.


return self

Expand Down
1 change: 0 additions & 1 deletion test-data/everest/egg/everest/model/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ wells:
controls:
-
name: well_rate
type: generic_control
min: 0
max: 1
perturbation_magnitude: 0.15
Expand Down
1 change: 0 additions & 1 deletion test-data/everest/egg/everest/model/config_flow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ wells:
controls:
-
name: well_rate
type: generic_control
min: 0
max: 1
perturbation_magnitude: 0.15
Expand Down
1 change: 0 additions & 1 deletion test-data/everest/math_func/config_advanced.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ controls:
min: -1.0
initial_guess: 0.25
perturbation_magnitude: 0.005
type: generic_control
variables:
- name: x
index: 0
Expand Down
1 change: 0 additions & 1 deletion test-data/everest/math_func/config_minimal.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
controls:
- name: point
type: generic_control
min: -1.0
max: 1.0
initial_guess: 0.1
Expand Down
1 change: 0 additions & 1 deletion test-data/everest/math_func/config_multiobj.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
controls:
-
name: point
type: generic_control
min: -1.0
max: 1.0
initial_guess: 0
Expand Down
2 changes: 0 additions & 2 deletions tests/everest/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ def _copy_tree(path: str | None = None):
def control_data_no_variables() -> dict[str, str | float]:
return {
"name": "group_0",
"type": "well_control",
"min": 0.0,
"max": 0.1,
"perturbation_magnitude": 0.005,
Expand Down Expand Up @@ -213,7 +212,6 @@ def min_config():
controls:
-
name: my_control
type: well_control
min: 0
max: 0.1
variables:
Expand Down
24 changes: 4 additions & 20 deletions tests/everest/test_config_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ def test_that_duplicate_control_names_raise_error():
controls=[
{
"name": "group_0",
"type": "well_control",
"min": 0,
"max": 0.1,
"variables": [
Expand All @@ -194,7 +193,6 @@ def test_that_duplicate_control_names_raise_error():
},
{
"name": "group_0",
"type": "well_control",
"min": 0,
"max": 0.1,
"variables": [
Expand All @@ -213,7 +211,6 @@ def test_that_dot_not_in_control_names():
controls=[
{
"name": "group_0.2",
"type": "well_control",
"min": 0,
"max": 0.1,
"variables": [
Expand All @@ -236,7 +233,6 @@ def test_that_scaled_range_is_valid_range():
controls=[
{
"name": "group_0",
"type": "well_control",
"min": 0,
"max": 0.1,
"scaled_range": [2, 1],
Expand Down Expand Up @@ -294,9 +290,7 @@ def test_that_invalid_control_initial_guess_outside_bounds(
):
with pytest.raises(ValueError) as e:
EverestConfig.with_defaults(
controls=[
{"name": "group_0", "type": "well_control", "variables": variables}
]
controls=[{"name": "group_0", "variables": variables}]
)

assert (
Expand Down Expand Up @@ -345,7 +339,6 @@ def test_that_invalid_control_unique_entry(variables, unique_key):
controls=[
{
"name": "group_0",
"type": "well_control",
"max": 0,
"min": 0.1,
"variables": variables,
Expand All @@ -365,7 +358,6 @@ def test_that_invalid_control_undefined_fields():
controls=[
{
"name": "group_0",
"type": "well_control",
"variables": [
{"name": "w00"},
],
Expand All @@ -386,7 +378,6 @@ def test_that_control_variables_index_is_defined_for_all_variables():
controls=[
{
"name": "group_0",
"type": "well_control",
"min": 0,
"max": 0.1,
"variables": [
Expand Down Expand Up @@ -998,7 +989,6 @@ def test_load_file_with_errors(copy_math_func_test_data_to_tmp, capsys):
content = file.read()

with open("config_minimal_error.yml", "w", encoding="utf-8") as file:
content = content.replace("generic_control", "yolo_control")
content = content.replace("max: 1.0", "max: not_a number")
pos = content.find("name: distance")
content = content[: pos + 14] + "\n invalid: invalid" + content[pos + 14 :]
Expand All @@ -1009,19 +999,13 @@ def test_load_file_with_errors(copy_math_func_test_data_to_tmp, capsys):
EverestConfig.load_file_with_argparser("config_minimal_error.yml", parser)

captured = capsys.readouterr()
assert "Found 2 validation error" in captured.err

assert "Found 3 validation error" in captured.err
assert "line: 3, column: 11" in captured.err
assert (
"Input should be 'well_control' or 'generic_control' (type=literal_error)"
in captured.err
)

assert "line: 5, column: 10" in captured.err
assert "line: 4, column: 10" in captured.err
assert (
"Input should be a valid number, unable to parse string as a number (type=float_parsing)"
in captured.err
)

assert "line: 16, column: 5" in captured.err
assert "line: 15, column: 5" in captured.err
assert "Extra inputs are not permitted (type=extra_forbidden)" in captured.err
4 changes: 1 addition & 3 deletions tests/everest/test_controls.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def test_controls_initialization():
config.controls.append(
ControlConfig(
name=exp_grp_name,
type="well_control",
variables=[
ControlVariableConfig(
name=a_ctrl_name,
Expand Down Expand Up @@ -84,7 +83,6 @@ def _perturb_control_zero(
if revised_control_zero is None:
revised_control_zero = ControlConfig(
name=control_zero.name,
type=control_zero.type,
min=gmin,
max=gmax,
initial_guess=ginit,
Expand Down Expand Up @@ -232,7 +230,7 @@ def test_control_none_well_variable_name():
config.controls[0].variables[0].name = illegal_name
with pytest.raises(
ValidationError,
match="Variable name does not match any well name",
match="Variable names should either all or none match well names",
):
EverestConfig.model_validate(config.to_dict())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
variables:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
variables:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
perturbation_magnitude : 0.005
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
perturbation_magnitude : 0.005
Expand Down
3 changes: 0 additions & 3 deletions tests/everest/test_data/mocked_test_case/config_samplers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
sampler:
Expand All @@ -21,7 +20,6 @@ controls:
initial_guess: 0.0624
-
name: group_1
type: well_control
min: 0
max: 0.1
variables:
Expand All @@ -40,7 +38,6 @@ controls:
method: uniform
-
name: group_2
type: well_control
min: 0
max: 0.1
sampler:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
perturbation_magnitude : 0.005
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
perturbation_magnitude : 0.005
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ wells:
controls:
-
name: group
type: well_control
min: 0
max: 0.1
perturbation_magnitude : 0.005
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ wells:

controls:
- name: well_priorities
type: well_control
min: 0.0
max: 1.0
perturbation_magnitude: 0.05
Expand All @@ -34,7 +33,6 @@ controls:
- { name: WELL-5, initial_guess: [0.52, 0.50, 0.52, 0.56] }

- name: swapping_constraints
type: generic_control
min: 0.0
max: 1.0
perturbation_magnitude: 0.05
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ wells:

controls:
- name: well_priorities
type: well_control
min: 0.0
max: 1.0
perturbation_magnitude: 0.05
Expand All @@ -49,7 +48,6 @@ controls:
- { name: WELL-5, index: 4, initial_guess: 0.56 }

- name: swapping_constraints
type: generic_control
min: 0.0
max: 1.0
perturbation_magnitude: 0.05
Expand Down
1 change: 0 additions & 1 deletion tests/everest/test_data/templating/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ wells:
controls:
-
name: well_drill
type: well_control
min: 0
max: 1
variables:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ wells:
controls:
-
name: well_order
type: well_control
min: 0
max: 1
perturbation_magnitude: 0.05
Expand All @@ -27,7 +26,6 @@ controls:

-
name: well_number
type: generic_control
min: 0.0
max: 1.0
perturbation_magnitude: 0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
controls:
-
name: not_really_important_for_this_test
type: generic_control
min: -1.0
max: 1.0
initial_guess: 0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
controls:
-
name: not_really_important_for_this_test
type: generic_control
min: -1.0
max: 1.0
initial_guess: 0
Expand Down
Loading
Loading