- 
                Notifications
    
You must be signed in to change notification settings  - Fork 120
 
Add grid properties #11851
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
Add grid properties #11851
Conversation
          
CodSpeed Performance ReportMerging #11851 will not alter performanceComparing  Summary
 Footnotes | 
    
1cbb8a3    to
    9c898c7      
    Compare
  
    | if global_grid_file_path is not None: | ||
| global_grid_file_path = Path(global_grid_file_path) | ||
| 
               | 
          ||
| grid_extension = global_grid_file_path.suffix.lower() | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could maybe all this grid relates parsing and loading be in the field config as classmethods?
4944055    to
    c281bc8      
    Compare
  
            
          
                src/ert/config/ensemble_config.py
              
                Outdated
          
        
      | f"Grid file {grid_file_path} did not contain dimensions", | ||
| grid_file_path, | ||
| ) | ||
| assert grid_file_path is not None | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can move this assert before line == 128 and just do assert global_grid_file_path is not None
| 
               | 
          ||
| if errors: | ||
| raise ConfigValidationError.from_collected(errors) | ||
| assert file_format is not None | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these 2 asserts be removed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I still need both asserts.
| corner_indices = [] | ||
| 
               | 
          ||
| if left_handed: | ||
| origin_cell = (1, 1, 1) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a very naive question :) The cells start with (1,1,1) and not (0,0,0) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best if @oddvarlia answers this I think.
| # Returns 24 values: [x0,y0,z0, x1,y1,z1, ..., x7,y7,z7] | ||
| coord = grid.get_xyz_cell_corners(ijk=corner_index, activeonly=False) | ||
| coord_cell.append(coord) | ||
| 
               | 
          
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion with vectorize approach:
coords = [np.array(grid.get_xyz_cell_corners(ijk=idx, activeonly=False)).reshape(8, 3)
          for idx in corner_indices]
if left_handed:
    origin = coords[0][0, :2] 
    xdir   = coords[1][1, :2] 
    ydir   = coords[2][2, :2] 
else:
    origin = coords[0][2, :2]
    xdir   = coords[1][3, :2]
    ydir   = coords[2][0, :2]
xlength = np.linalg.norm(xdir - origin)
ylength = np.linalg.norm(ydir - origin)
xinc = xlength / nx
yinc = ylength / ny692c065    to
    bf27eac      
    Compare
  
    bf27eac    to
    f6b566e      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice PR @dafeda !
As discussed about the commit message content:
- localization params introduction via xtgeo (more verbose) and it's called ertbox
 - maybe something on ertbox?
 - storage migration to14 with some test modification
 
Issue
Resolves #10909
Example of usage via API
Field(type='field', name='COND', forward_init=True, update=True, nx=10, ny=10, nz=1, xlength=10.0, ylength=10.0, xinc=1.0, yinc=1.0, rotation_angle=0.0, origin=(6.123233998228043e-16, 10.0), file_format=<FieldFileFormat.BGRDECL: 'bgrdecl'>, output_transformation=None, input_transformation=None, truncation_min=None, truncation_max=None, forward_init_file='cond.bgrdecl', output_file=PosixPath('cond.bgrdecl'), grid_file='/Users/FCUR/git/ert/test-data/ert/heat_equation/CASE.EGRID', mask_file=PosixPath('/Users/FCUR/git/ert/test-data/ert/heat_equation/storage/experiments/f094b1d1-de8d-4dff-aaa3-2503bd2626d1/grid_mask.npy'))
Performance (ran locally on my mac)
Drogon (approx 900k parameters):
Time to load grid and calculate ertbox params: 0.0660250186920166s
ErtboxParameters(nx=92, ny=146, nz=66, xlength=92.0, ylength=146.0, xinc=1.0, yinc=1.0, rotation_angle=0.0, origin=(0.0, 146.0))
10mill parameters:
Time to load grid and calculate ertbox params: 1.0769238471984863s
ErtboxParameters(nx=1000, ny=100, nz=100, xlength=1000.0, ylength=100.0, xinc=1.0, yinc=1.0, rotation_angle=0.0, origin=(0.0, 100.0))
git rebase -i main --exec 'just rapid-tests')When applicable