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.

Sign up for GitHub

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

Fb asim 4672 missing emulsion models #247

Merged
merged 15 commits into from
May 20, 2022
Merged

Fb asim 4672 missing emulsion models #247

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9c39ff0
Update relative viscosity model api
May 11, 2022
85bb5a2
New relative viscosity models and their parameters
May 11, 2022
6982619
Update hook_specs.py
May 11, 2022
cfeedc3
Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…
May 13, 2022
2a006e1
Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…
May 17, 2022
9754276
Update changelog
May 18, 2022
47cda3b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 18, 2022
07dddfe
Update changelog
May 18, 2022
5e8041a
Suggestions from previous code review
May 18, 2022
19bc185
Update changelog
May 18, 2022
c40eccc
Code review:
May 20, 2022
794e670
Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…
May 20, 2022
f828fd5
Update code generated
May 20, 2022
af1206f
Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…
May 20, 2022
0aaaa1d
fix docstring
May 20, 2022
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
14 changes: 14 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ History
emulsion_model_enabled: True
emulsion_inversion_point_model: brinkman1952_and_yeh1964

* **Breaking Change**: change signature of hook ``calculate_relative_emulsion_viscosity``. This hook is also receiving the fluid temperature and a flag indicating whether the dispersion is water in oil or oil in water;

* Add new emulsion relative viscosity models:

- ``pal_rhodes1989``;
- ``ronningsen1995``;
- ``volumetric_weight``;
- ``woelflin_1942``;
- ``barnea_mizrahi1976``;
- ``table_based``;

* Add emulsion constant inversion point model;

* Add emulsion relative viscosity tuning factor;
Copy link
Member

Choose a reason for hiding this comment

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

Nice!
I forgot to ask you to add this entry in the previous PR.


0.13.0 (2022-04-19)
===================
Expand Down
20 changes: 20 additions & 0 deletions docs/source/alfacase_definitions/PhysicsDescription.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
keep_former_results: bool = False
emulsion_model_enabled: bool = True
emulsion_relative_viscosity_model: \ :class:`EmulsionRelativeViscosityModelType <alfasim_sdk._internal.constants.EmulsionRelativeViscosityModelType>`\ = EmulsionRelativeViscosityModelType.ModelDefault
emulsion_pal_rhodes_phi_rel_100: \ :class:`Scalar <barril.units.Scalar>`\ = Scalar(0.765, '-', 'dimensionless')
emulsion_woelflin_a: \ :class:`Scalar <barril.units.Scalar>`\ = Scalar(4.2, '-', 'dimensionless')
emulsion_woelflin_b: \ :class:`Scalar <barril.units.Scalar>`\ = Scalar(2.5, '-', 'dimensionless')
emulsion_table_based_rel_visc_curve: \ :class:`Curve <barril.curve.curve.Curve>`\ = Curve(-, m3/m3)[(1.0, 0.0)]
emulsion_relative_viscosity_tuning_factor: \ :class:`Curve <barril.curve.curve.Curve>`\ = Curve(-, m3/m3)[(1.0, 0.0)]
emulsion_droplet_size_model: \ :class:`EmulsionDropletSizeModelType <alfasim_sdk._internal.constants.EmulsionDropletSizeModelType>`\ = EmulsionDropletSizeModelType.ModelDefault
emulsion_inversion_point_model: \ :class:`EmulsionInversionPointModelType <alfasim_sdk._internal.constants.EmulsionInversionPointModelType>`\ = EmulsionInversionPointModelType.ModelDefault
Expand All @@ -36,6 +40,22 @@
keep_former_results: boolean # optional
emulsion_model_enabled: boolean # optional
emulsion_relative_viscosity_model: \ :class:`EmulsionRelativeViscosityModelType <alfasim_sdk._internal.constants.EmulsionRelativeViscosityModelType>`\ # optional
emulsion_pal_rhodes_phi_rel_100: # optional
value: number
unit: string
emulsion_woelflin_a: # optional
value: number
unit: string
emulsion_woelflin_b: # optional
value: number
unit: string
emulsion_table_based_rel_visc_curve: # optional
image:
values: [number]
unit: string
domain:
values: [number]
unit: string
emulsion_relative_viscosity_tuning_factor: # optional
image:
values: [number]
Expand Down
7 changes: 4 additions & 3 deletions src/alfasim_sdk/_internal/alfacase/alfacase_to_case.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,6 @@ def get_curve_loader(
domain_category, from_domain_unit
),
)
assert category is not None or from_unit is not None, (
"At least the category or the unit must be provided in " "get_curve_loader"
)
return partial(
load_curve,
category=_obtain_category_for_scalar(category, from_unit),
Expand Down Expand Up @@ -1925,6 +1922,10 @@ def load_physics_description(
'emulsion_model': get_enum_loader(enum_class=constants.EmulsionModelType),
'emulsion_model_enabled': load_value,
'emulsion_relative_viscosity_model': get_enum_loader(enum_class=constants.EmulsionRelativeViscosityModelType),
'emulsion_pal_rhodes_phi_rel_100': get_scalar_loader(category='dimensionless'),
'emulsion_woelflin_a': get_scalar_loader(category='dimensionless'),
'emulsion_woelflin_b': get_scalar_loader(category='dimensionless'),
'emulsion_table_based_rel_visc_curve': get_curve_loader(category='dimensionless', domain_category='volume per volume'),
'emulsion_relative_viscosity_tuning_factor': get_curve_loader(category='dimensionless', domain_category='volume per volume'),
'emulsion_droplet_size_model': get_enum_loader(enum_class=constants.EmulsionDropletSizeModelType),
'emulsion_inversion_point_model': get_enum_loader(enum_class=constants.EmulsionInversionPointModelType),
Expand Down
14 changes: 12 additions & 2 deletions src/alfasim_sdk/_internal/alfacase/case_description.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2645,6 +2645,17 @@ class PhysicsDescription:
emulsion_relative_viscosity_model = attrib_enum(
default=constants.EmulsionRelativeViscosityModelType.ModelDefault
)
emulsion_pal_rhodes_phi_rel_100 = attrib_scalar(
default=Scalar("dimensionless", 0.765, "-")
)
emulsion_woelflin_a = attrib_scalar(default=Scalar("dimensionless", 4.2, "-"))
emulsion_woelflin_b = attrib_scalar(default=Scalar("dimensionless", 2.5, "-"))
emulsion_table_based_rel_visc_curve = attrib_curve(
default=Curve(
image=Array("dimensionless", [1.0], "-"),
domain=Array("volume per volume", [0.0], "m3/m3"),
)
)

emulsion_relative_viscosity_tuning_factor = attrib_curve(
default=Curve(
Expand Down Expand Up @@ -2679,13 +2690,12 @@ def _validate_inversion_point_water_cut(self, attribute, value):

@emulsion_relative_viscosity_tuning_factor.validator
def _validate_emulsion_relative_viscosity_tuning_factor(self, attribute, value):
assert isinstance(value, Curve), "Invalid tuning factor curve"
domain = value.GetDomain()
assert domain.GetCategory() == "volume per volume", "Invalid water-cut category"
domain_values = np.asarray(domain.GetValues("m3/m3"))
assert (
np.min(domain_values) >= 0.0 and np.max(domain_values) <= 1.0
), "Invlid water-cut values"
), "Invalid water-cut values"
image = value.GetImage()
assert (
image.GetCategory() == "dimensionless"
Expand Down
13 changes: 11 additions & 2 deletions src/alfasim_sdk/_internal/alfacase/schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,16 @@
Optional("restart_filepath"): Str(),
Optional("keep_former_results"): Bool(),
Optional("emulsion_model_enabled"): Bool(),
Optional("emulsion_relative_viscosity_model"): Enum(['model_default', 'taylor1932', 'brinkman1952', 'mooney1951a', 'mooney1951b', 'from_plugin']),
Optional("emulsion_relative_viscosity_model"): Enum(['model_default', 'taylor1932', 'brinkman1952', 'mooney1951a', 'mooney1951b', 'from_plugin', 'pal_rhodes1989', 'ronningsen1995', 'volumetric_weight', 'woelflin_1942', 'barnea_mizrahi1976', 'table_based']),
Optional("emulsion_pal_rhodes_phi_rel_100"): Map({"value": Float(), "unit": Str()}),
Optional("emulsion_woelflin_a"): Map({"value": Float(), "unit": Str()}),
Optional("emulsion_woelflin_b"): Map({"value": Float(), "unit": Str()}),
Optional("emulsion_table_based_rel_visc_curve"): Map(
{
"image": Map({"values": Seq(Float()), "unit": Str()}),
"domain": Map({"values": Seq(Float()), "unit": Str()}),
}
),
Optional("emulsion_relative_viscosity_tuning_factor"): Map(
{
"image": Map({"values": Seq(Float()), "unit": Str()}),
Expand Down Expand Up @@ -1103,5 +1112,5 @@
Optional("walls"): Seq(wall_description_schema),
}
)
# [[[end]]] (checksum: 890908f4d703c94a0ead1cd310571777)
# [[[end]]] (checksum: 33a59c90d11bcb722e862ee6d79ec8b4)
# fmt: on
6 changes: 6 additions & 0 deletions src/alfasim_sdk/_internal/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ class EmulsionRelativeViscosityModelType(Enum):
Mooney1951a = "mooney1951a"
Mooney1951b = "mooney1951b"
FromPlugin = "from_plugin"
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to put FromPlugin option as the last one.
But I am not sure if it will break something with the Enum.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this would require some treatment when loading old cases. I don't know if it is worthy to do such modification

Copy link
Member

Choose a reason for hiding this comment

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

We save the "string" not some auto index or some thing like that.
That being said, adding new values (any where in the enum) will not affect loading old cases.
Changing values (the strings) or removing the entries will affect old cases using the old values.

Copy link
Member

Choose a reason for hiding this comment

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

Nice! Thanks for the explanation @prusse-martin!

Copy link
Member

Choose a reason for hiding this comment

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

But I also prefer FromPlugin kind placed apart from the others, like last or first.
Having it as first has the benefit of getting out of change sets, I will put as a possible problem with that that it will be the default value, but on case description no default is assumed for enums.
That will probably be an issue with alfasim, where every thing has defaults and possibly some enum use implicit defaults (the "first value") but that can be fixed without backward compatibility effects and easily detectable checking the status.

Copy link
Contributor Author

@edersongrein edersongrein May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

You are right @prusse-martin.

  • Replace plugin entry

PalRhodes1989 = "pal_rhodes1989"
Ronningsen1995 = "ronningsen1995"
VolumetricWeight = "volumetric_weight"
Woelflin1942 = "woelflin_1942"
BarneaMizrahi1976 = "barnea_mizrahi1976"
TableBased = "table_based"


class EmulsionDropletSizeModelType(Enum):
Expand Down
2 changes: 2 additions & 0 deletions src/alfasim_sdk/_internal/hook_specs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,8 @@ def calculate_relative_emulsion_viscosity(
mu_disp: "double",
mu_cont: "double",
alpha_disp_in_layer: "double",
T: "double",
Copy link
Member

@prusse-martin prusse-martin May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think that changing hook signatures will cause incompatibility with existing plugins.
And they will need to be updated (source code changed, not just recompiled).
Changelog entry about the breaking change?
Or we could go with creating an alternative version for the hook like calculate_relative_emulsion_viscosity_per_temperature or some thing else in those lines (does T stands for temperature?).

I also think you should update the docstring to properly document the signature, I think that is what goes to the online documentation.

Copy link
Contributor Author

@edersongrein edersongrein May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This will break the existing plugins indeed and I forgot to update changelog and the docstring.

  • Update changelog and docstring

Or we could go with creating an alternative version for the hook like calculate_relative_emulsion_viscosity_per_temperature or some thing else in those lines (does T stands for temperature?

Yes, T is for temperature and creating another hook is not viable. Fortunately as far as I know there is nobody using this plugin, so this change probably won't bother anyone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact I have updated the changelog. It was only the docstring that was missed

Copy link
Member

Choose a reason for hiding this comment

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

... creating another hook is not viable ... there is nobody using this plugin...

OK... I will let it pass for now =P

water_in_oil: "bool",
) -> "int":
"""
**c++ signature** : ``HOOK_RELATIVE_EMULSION_VISCOSITY(void* ctx, double* mu_r, double mu_disp, double mu_cont, double alpha_disp_in_layer)``
Expand Down
6 changes: 5 additions & 1 deletion src/alfasim_sdk/alfasim_sdk_api/api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,14 +908,18 @@ DLL_EXPORT int get_liq_liq_shear_force_per_volume_input_variable(
@param[in] mu_disp Dispersed Field Viscosity [Pa.s].
@param[in] mu_cont Continuous Field Viscosity [Pa.s].
@param[in] alpha_disp_in_layer Dispersed Field Volume Fraction in the layer (emulsion) [m3 of dispersed field /m3 of layer].
@param[in] T Fluid temperature [K].
@param[in] water_in_oil True when water is dispersed in oil, otherwise it is a dispersion of oil in water.
@return An #error_code value.
*/
DLL_EXPORT int get_relative_emulsion_viscosity(
void* ctx,
double* out,
double mu_disp,
double mu_cont,
double alpha_disp_in_layer
double alpha_disp_in_layer,
Copy link
Member

@prusse-martin prusse-martin May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Related to {{comment on hook_specs.py}}.

double T,
bool water_in_oil
);

#endif
2 changes: 1 addition & 1 deletion src/alfasim_sdk/alfasim_sdk_api/detail/api_pointers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ typedef int (*get_tracer_partition_coefficient_func)(void* ctx, double* out, voi
typedef int (*get_plugin_input_data_multiplereference_selected_size_func)(void* ctx, int* indexes_size, const char* plugin_id, const char* var_name);
typedef int (*get_input_variable_func)(void* ctx, double* out, const char* var_name, int phase_id);
typedef int (*get_ucm_fluid_geometrical_properties_func)(void* ctx, double* S_w, double* S_i, double* H, double alpha_G, double D);
typedef int (*get_relative_emulsion_viscosity_func)(void* ctx, double* out, double mu_disp, double mu_cont, double alpha_disp_in_layer, int disp_field_id, int cont_field_id);
typedef int (*get_relative_emulsion_viscosity_func)(void* ctx, double* out, double mu_disp, double mu_cont, double alpha_disp_in_layer, double T, bool water_in_oil);
Copy link
Member

@prusse-martin prusse-martin May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Related to {{comment on hook_specs.py}}.


struct ALFAsimSDK_API {
#if defined(_WIN32)
Expand Down
20 changes: 20 additions & 0 deletions ...se_description_docstring/test_generate_case_description_docstring_PhysicsDescription_.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
keep_former_results: bool = False
emulsion_model_enabled: bool = True
emulsion_relative_viscosity_model: \ :class:`EmulsionRelativeViscosityModelType <alfasim_sdk._internal.constants.EmulsionRelativeViscosityModelType>`\ = EmulsionRelativeViscosityModelType.ModelDefault
emulsion_pal_rhodes_phi_rel_100: \ :class:`Scalar <barril.units.Scalar>`\ = Scalar(0.765, '-', 'dimensionless')
emulsion_woelflin_a: \ :class:`Scalar <barril.units.Scalar>`\ = Scalar(4.2, '-', 'dimensionless')
emulsion_woelflin_b: \ :class:`Scalar <barril.units.Scalar>`\ = Scalar(2.5, '-', 'dimensionless')
emulsion_table_based_rel_visc_curve: \ :class:`Curve <barril.curve.curve.Curve>`\ = Curve(-, m3/m3)[(1.0, 0.0)]
emulsion_relative_viscosity_tuning_factor: \ :class:`Curve <barril.curve.curve.Curve>`\ = Curve(-, m3/m3)[(1.0, 0.0)]
emulsion_droplet_size_model: \ :class:`EmulsionDropletSizeModelType <alfasim_sdk._internal.constants.EmulsionDropletSizeModelType>`\ = EmulsionDropletSizeModelType.ModelDefault
emulsion_inversion_point_model: \ :class:`EmulsionInversionPointModelType <alfasim_sdk._internal.constants.EmulsionInversionPointModelType>`\ = EmulsionInversionPointModelType.ModelDefault
Expand All @@ -36,6 +40,22 @@
keep_former_results: boolean # optional
emulsion_model_enabled: boolean # optional
emulsion_relative_viscosity_model: \ :class:`EmulsionRelativeViscosityModelType <alfasim_sdk._internal.constants.EmulsionRelativeViscosityModelType>`\ # optional
emulsion_pal_rhodes_phi_rel_100: # optional
value: number
unit: string
emulsion_woelflin_a: # optional
value: number
unit: string
emulsion_woelflin_b: # optional
value: number
unit: string
emulsion_table_based_rel_visc_curve: # optional
image:
values: [number]
unit: string
domain:
values: [number]
unit: string
emulsion_relative_viscosity_tuning_factor: # optional
image:
values: [number]
Expand Down
11 changes: 10 additions & 1 deletion tests/alfacase/test_generate_case_schema/test_generate_schema_for_all_cases.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,16 @@ physics_description_schema = Map(
Optional("restart_filepath"): Str(),
Optional("keep_former_results"): Bool(),
Optional("emulsion_model_enabled"): Bool(),
Optional("emulsion_relative_viscosity_model"): Enum(['model_default', 'taylor1932', 'brinkman1952', 'mooney1951a', 'mooney1951b', 'from_plugin']),
Optional("emulsion_relative_viscosity_model"): Enum(['model_default', 'taylor1932', 'brinkman1952', 'mooney1951a', 'mooney1951b', 'from_plugin', 'pal_rhodes1989', 'ronningsen1995', 'volumetric_weight', 'woelflin_1942', 'barnea_mizrahi1976', 'table_based']),
Optional("emulsion_pal_rhodes_phi_rel_100"): Map({"value": Float(), "unit": Str()}),
Optional("emulsion_woelflin_a"): Map({"value": Float(), "unit": Str()}),
Optional("emulsion_woelflin_b"): Map({"value": Float(), "unit": Str()}),
Optional("emulsion_table_based_rel_visc_curve"): Map(
{
"image": Map({"values": Seq(Float()), "unit": Str()}),
"domain": Map({"values": Seq(Float()), "unit": Str()}),
}
),
Optional("emulsion_relative_viscosity_tuning_factor"): Map(
{
"image": Map({"values": Seq(Float()), "unit": Str()}),
Expand Down