Fb asim 4672 missing emulsion models #247

edersongrein · 2022-05-18T11:38:19Z

No description provided.

…g-emulsion-models # Conflicts: # docs/source/alfacase_definitions/PhysicsDescription.txt # src/alfasim_sdk/_internal/alfacase/alfacase_to_case.py # src/alfasim_sdk/_internal/alfacase/case_description.py # src/alfasim_sdk/_internal/alfacase/schema.py # tests/alfacase/test_generate_case_description_docstring/test_generate_case_description_docstring_PhysicsDescription_.txt # tests/alfacase/test_generate_case_schema/test_generate_schema_for_all_cases.txt

ASIM-4672

for more information, see https://pre-commit.ci

codecov · 2022-05-18T11:39:45Z

Codecov Report

Merging #247 (0aaaa1d) into master (c56e8eb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #247   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files          41       41           
  Lines        4455     4457    +2     
=======================================
+ Hits         4453     4455    +2     
  Misses          2        2

ASIM-4672

ggrbill · 2022-05-18T19:32:18Z

CHANGELOG.rst

+
+* Add emulsion constant inversion point model;
+
+* Add emulsion relative viscosity tuning factor;


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

ggrbill · 2022-05-18T19:34:00Z

src/alfasim_sdk/_internal/constants.py

@@ -115,6 +115,12 @@ class EmulsionRelativeViscosityModelType(Enum):
    Mooney1951a = "mooney1951a"
    Mooney1951b = "mooney1951b"
    FromPlugin = "from_plugin"


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

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

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.

Nice! Thanks for the explanation @prusse-martin!

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.

You are right @prusse-martin.

Replace plugin entry

prusse-martin · 2022-05-19T17:25:51Z

src/alfasim_sdk/_internal/hook_specs.py

@@ -1785,6 +1785,8 @@ def calculate_relative_emulsion_viscosity(
    mu_disp: "double",
    mu_cont: "double",
    alpha_disp_in_layer: "double",
+    T: "double",


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.

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

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

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

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

prusse-martin · 2022-05-19T17:27:32Z

src/alfasim_sdk/alfasim_sdk_api/api.h

    @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,


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

prusse-martin · 2022-05-19T17:27:39Z

src/alfasim_sdk/alfasim_sdk_api/detail/api_pointers.h

@@ -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);


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

prusse-martin · 2022-05-19T17:29:53Z

src/alfasim_sdk/_internal/constants.py

@@ -115,6 +115,12 @@ class EmulsionRelativeViscosityModelType(Enum):
    Mooney1951a = "mooney1951a"
    Mooney1951b = "mooney1951b"
    FromPlugin = "from_plugin"


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.

Enum entry replaced Emulsion model docstring updated ASIM-4672

Ederson Augusto Grein and others added 7 commits May 11, 2022 15:12

Update relative viscosity model api

9c39ff0

ASIM-4672

New relative viscosity models and their parameters

85bb5a2

ASIM-4672

Update hook_specs.py

6982619

Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…

cfeedc3

…g-emulsion-models

Update changelog

9754276

ASIM-4672

[pre-commit.ci] auto fixes from pre-commit.com hooks

47cda3b

for more information, see https://pre-commit.ci

Ederson Augusto Grein added 2 commits May 18, 2022 08:41

Update changelog

07dddfe

ASIM-4672

Suggestions from previous code review

5e8041a

ASIM-4672

edersongrein requested review from prusse-martin and ggrbill May 18, 2022 16:53

Update changelog

19bc185

ASIM-4672

ggrbill approved these changes May 18, 2022

View reviewed changes

prusse-martin requested changes May 19, 2022

View reviewed changes

Code review:

c40eccc

Enum entry replaced Emulsion model docstring updated ASIM-4672

edersongrein requested a review from prusse-martin May 20, 2022 11:12

Ederson Augusto Grein added 3 commits May 20, 2022 08:16

Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…

794e670

…g-emulsion-models

Update code generated

f828fd5

ASIM-4672

Merge remote-tracking branch 'origin/master' into fb-ASIM-4672-missin…

af1206f

…g-emulsion-models

prusse-martin approved these changes May 20, 2022

View reviewed changes

fix docstring

0aaaa1d

ASIM-4672

edersongrein merged commit ee2fb88 into master May 20, 2022

edersongrein deleted the fb-ASIM-4672-missing-emulsion-models branch May 20, 2022 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fb asim 4672 missing emulsion models #247

Fb asim 4672 missing emulsion models #247

edersongrein commented May 18, 2022

codecov bot commented May 18, 2022 •

edited

Loading

ggrbill May 18, 2022

ggrbill May 18, 2022

edersongrein May 19, 2022

prusse-martin May 19, 2022

ggrbill May 19, 2022

prusse-martin May 19, 2022

edersongrein May 20, 2022 •

edited

Loading

prusse-martin May 19, 2022 •

edited

Loading

edersongrein May 20, 2022 •

edited

Loading

edersongrein May 20, 2022

prusse-martin May 20, 2022

prusse-martin May 19, 2022 •

edited

Loading

prusse-martin May 19, 2022 •

edited

Loading

prusse-martin May 19, 2022


		* Add emulsion constant inversion point model;

		* Add emulsion relative viscosity tuning factor;

Fb asim 4672 missing emulsion models #247

Fb asim 4672 missing emulsion models #247

Conversation

edersongrein commented May 18, 2022

codecov bot commented May 18, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edersongrein May 20, 2022 • edited Loading

Choose a reason for hiding this comment

prusse-martin May 19, 2022 • edited Loading

Choose a reason for hiding this comment

edersongrein May 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prusse-martin May 19, 2022 • edited Loading

Choose a reason for hiding this comment

prusse-martin May 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 18, 2022 •

edited

Loading

edersongrein May 20, 2022 •

edited

Loading

prusse-martin May 19, 2022 •

edited

Loading

edersongrein May 20, 2022 •

edited

Loading

prusse-martin May 19, 2022 •

edited

Loading

prusse-martin May 19, 2022 •

edited

Loading