feat: Add Well Flow Indicator function

[johanlrdahl]

Adding function "calculate_xmt_prod_status" for Well Flow Indication

Description

New function added to define if well is open

Motivation and Context

See Jira Issue AH-1895

How Has This Been Tested?

Added 8 tests to pytest

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Refactor (non-breaking change which improves implementation)
• Performance (non-breaking change which improves performance. Please add associated performance test and results)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Non-functional change (xml comments/documentation/etc)

Contributor Checklist:

• My code follows the code style of this project.
• I have added an example of my new feature and included it in the documentation.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• My Pull Request name follows the naming convention fix: <description>, feat: <description>, etc.

Reviewer Checklist for Charts compliant functions:

• The docstrings of the new function follow the contributing guidelines.
• The new function is professionally documented
• The new function and associated scripts are covered by one or more unit tests and code coverage did not decrease.
• The new function is accompanied by an example and it is included in the Gallery of Charts.
• The new function is reviewed in Chromatic. Access the storybook build results url and comment, approve or deny.
• All function inputs, arguments, and outputs have a supported data type and have human readable names.
• No code language is included in the description of the function or parameters (e.g use "polynomial order" instead of "poly_order")

[codecov]

Codecov Report

Merging #125 (4ad6084) into main (0c73fc3) will increase coverage by 0.03%.
The diff coverage is 97.43%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
+ Coverage   91.33%   91.37%   +0.03%     
==========================================
  Files         105      105              
  Lines        3808     3837      +29     
  Branches      822      832      +10     
==========================================
+ Hits         3478     3506      +28     
  Misses        206      206              
- Partials      124      125       +1     

Files	Coverage Δ
indsl/equipment/valve_parameters.py	86.66% <100.00%> (-0.44%)	⬇️
indsl/equipment/valve_parameters_.py	89.47% <100.00%> (ø)
indsl/resample/auto_align.py	95.23% <100.00%> (ø)
indsl/oil_and_gas/well_prod_status.py	98.21% <96.77%> (-1.79%)	⬇️

[github-actions]

Unit Test Results

    21 files  ±  0      21 suites  ±0   53m 48s ⏱️ + 2m 28s
 1 103 tests +  8   1 103 ✅ +  8   0 💤 ±0  0 ❌ ±0 
15 651 runs  +168  15 640 ✅ +169  11 💤  - 1  0 ❌ ±0 

Results for commit 4ad6084. ± Comparison against base commit 0c73fc3.

This pull request removes 5 and adds 13 tests. Note that renamed tests count towards both.

tests.oil_and_gas.test_well_prod_status ‑ test_all_valve_combos
tests.oil_and_gas.test_well_prod_status ‑ test_empty_valve
tests.oil_and_gas.test_well_prod_status ‑ test_high_choke_threshold
tests.oil_and_gas.test_well_prod_status ‑ test_raise_error_threshold
tests.oil_and_gas.test_well_prod_status ‑ test_zero2one_valve_range

tests.oil_and_gas.test_well_prod_status.TestWellProdStatus ‑ test_all_valve_combos
tests.oil_and_gas.test_well_prod_status.TestWellProdStatus ‑ test_empty_valve
tests.oil_and_gas.test_well_prod_status.TestWellProdStatus ‑ test_high_choke_threshold
tests.oil_and_gas.test_well_prod_status.TestWellProdStatus ‑ test_raise_error_threshold
tests.oil_and_gas.test_well_prod_status.TestWellProdStatus ‑ test_zero2one_valve_range
tests.oil_and_gas.test_well_prod_status.TestXmtProdStatus ‑ test_all_valve_combinations
tests.oil_and_gas.test_well_prod_status.TestXmtProdStatus ‑ test_empty_series
tests.oil_and_gas.test_well_prod_status.TestXmtProdStatus ‑ test_merge_valves
tests.oil_and_gas.test_well_prod_status.TestXmtProdStatus ‑ test_merge_valves_empty_list
tests.oil_and_gas.test_well_prod_status.TestXmtProdStatus ‑ test_missing_all_wellhead_valves
…

♻️ This comment has been updated with latest results.

[johanlrdahl]

@simonecasolo when you review this, keep this in mind:

• The changes in the valve_parameters files were only to fix typing issues
• I organised the old pytests for "calculate_well_prod_status" in a separate Class so that we have two different classes for that function and our new "calculate_xmt_prod_status". But because of indentation and the addition of the "cls"-parameters for each function makes it look like I changes all the old tests. These are however unchanged.
• Please consider if any variable names can be improved. I do not know well flows very well. F.ex, I grouped master, annulus and xover-valves as "wellhead_valves" but if there is a better name for this or other variables, please give a better suggestion.
• I did not change the documentation as I am unsure how you want it to be. Should we remove the old "calculate_well_prod_status" from the doc but keep it as-is for now?

We can also go through the code and tests together later when you have read through the changes

[neringaalt]

@CogniteJohan @simonecasolo
Is the new function going to be exposed to Charts UI?

[johanlrdahl]

@simonecasolo , @skepticalcat I assume this is not supposed to be in Charts?

[simonecasolo]

Good question. I would say not for now

…

On Fri, 15 Mar 2024, 08:55 Johan Løwig Rambech Dahl, < ***@***.***> wrote: @simonecasolo <https://github.com/simonecasolo> , @skepticalcat <https://github.com/skepticalcat> I assume this is not supposed to be in Charts? — Reply to this email directly, view it on GitHub <#125 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APUAITKPW72BNBHTPE3DW7TYYKSQBAVCNFSM6AAAAABDKG7CQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJZGEYTIMJRGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[johanlrdahl]

@neringaalt As Simone commented, we keep this out of Charts for now. I would like @skepticalcat to have another review before be merge this into master. I have not updated documentation or added an example as I am unsure how to best do that. This is a change of some older function, but the name has changed so I guess documentation should be changed somewhat. Would love your input here @skepticalcat or @simonecasolo

[skepticalcat]

@CogniteJohan, @simonecasolo I think we can't remove the old well production function that easily because it is exposed to charts and there might be some charts using that.
So we'd either have to replace the old code with the new one, or, if we keep the old function, we would need to change the docstrings of the function slightly to indicate the differences between those two algorithms.

Even if the second one is not exposed to charts, it'll show up in the auto-generated docs, and currently we would have two very similar functions, with almost identical docs, but different names.

So, for now I'd suggest to change the docs for the xmt function to indicate the differences. Code looks great though!

[skepticalcat]

lgtm! Well (heh) done!