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

DRAFT: wellspec with dtop dbot #751

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

poc11
Copy link
Contributor

@poc11 poc11 commented Aug 25, 2023

I recently encountered a model that used the DTOP and DBOT nexus keywords to define a well.

WELLSPEC A_20-1
IW JW L KH RADW SKIN RADB WI PPERF DTOP DBOT
1 1 NA NA 0.2500 0.0100 NA NA 0.0 6325.0 6330.0
1 1 NA NA 0.2500 0.0200 NA NA 1.0 8332.0 8370.0

This creates an issue when trying to derive a BlockedWell from a wellspec. Specifically, when BlockedWell.derive_from_wellspec calls BlockedWell.derive_from_dataframe, there will not be any information in the dataframe's L (layer) column to populate the cell_kji0 = BlockedWell.__cell_kji0_from_df(df, i) variable.

Proposed fix as follows:
BlockedWell.dtop_dbot_to_layers function converts the existing DTOP/DBOTcolumns in the wellspec dataframe into corresponding k-layers. It does this by:
-loading corp = grid.corner_points()
-iterating through nk and checking if the k,j,i min and max depth is between is in between DTOP and DBOT
-if cell is in the DTOP-DBASE span, a new row is added to the modified dataframe with the corresponding L = k+1
-returns modified dataframe with corresponding L = k+1 values

This function is only called if there are non-null values in the DTOPand DBOT columns.

Additionally, resqpy.well.well_utils._derive_from_wellspec_verify_col_list function has been updated to provide DTOP and DBOT as defaults (along with IW, JW, L, ANGLA, ANGLV). This ensures the DTOP and DBOT columns are checked every time a well is loaded from wellspec.`

@poc11
Copy link
Contributor Author

poc11 commented Aug 25, 2023

@andy-beer
Can you have a look at this pull request? First time i've seen DTOP and DBOT used in a nexus deck but it is a legit Nexus keyword.

Have noticed you avoid caching the full cornerpoint array like i did in the dtop_dbot_to_layers function. Curious if you're ok with how I pulled the whole corp in corp = grid.corner_points() or if there are any better ways to do that.

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage: 16.66% and project coverage change: -0.05% ⚠️

Comparison is base (b072dc8) 81.06% compared to head (33c642a) 81.01%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #751      +/-   ##
==========================================
- Coverage   81.06%   81.01%   -0.05%     
==========================================
  Files         190      190              
  Lines       32529    32552      +23     
==========================================
+ Hits        26370    26373       +3     
- Misses       6159     6179      +20     
Files Changed Coverage Δ
resqpy/well/_blocked_well.py 81.98% <13.04%> (-1.00%) ⬇️
resqpy/well/well_utils.py 95.06% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dbot = data['DBOT']
j,i = int(data['JW']), int(data['IW'])
for k in range(nk):
if dbot == 'BOT':
Copy link
Contributor

Choose a reason for hiding this comment

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

does DTOP need to be checked when 'BOT' is in use for DBOT? (I haven't looked in the manual!)

if dbot == 'BOT':
data['L'] = k + 1
df_expanded.append(data.copy())
elif (corp[k,j,i,0,:,:,2].min() >= dtop) and (corp[k,j,i,1,:,:,2].max() <= dbot):
Copy link
Contributor

Choose a reason for hiding this comment

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

This will include cells that are fully within the dtop to dbot depth range. It would be better if we included cells that are partially in the range, though ideally with a PPERF being set too. Also, it might be slightly better to use the K-, K+ face mid points, rather than the corner points, as the definition of the cell depth range for this purpose.

The corp z units might be increasing upwards (ie. negative values), in which case the z values should be negated before applying this test. You can check the z axis direction in the boolean attribute: grid.crs.z_inc_down which is usually True but not always.

@andy-beer
Copy link
Contributor

Thanks for picking up on this missing functionality Patrick, and for working up the fix. A few comments responding to your questions and other thoughts that came to me:

  • The Grid.corner_points() method does have an optional cache_cp_array argument which defaults to False but can be used to cache a copy of the corner point array in the Grid object, if appropriate.
  • Creating the full corner point array can be a bit slow and very memory intensive. For this WELLSPEC functionality, it might be better to call it repeatedly for one cell at a time, by passing the optional cell_kji0 argument, which causes it to only do the computations for individual cells rather than the whole array.
  • You might consider using the Grid.face_centre() method instead of corner_points(), with the K face centres being used to define the depth range of a cell.
  • If we are not setting PPERF, and having an all-or-nothing approach to each cell, then an alternative would be to use the Grid.centre() method and decide a cell's inclusion based solely on the depth of the centre point.
  • We have a very stringent unit test code coverage regime these days – any changes need to be accompanied with unit tests to at least match the previous average coverage (currently around 81%).

WELLSPEC is a Nexus keyword. Nexus is a trademark of Halliburton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants