Skip to content

feat: Change to local Newell's normal#3977

Open
jafranc wants to merge 10 commits intodevelopfrom
jafranc/feat/localNewell
Open

feat: Change to local Newell's normal#3977
jafranc wants to merge 10 commits intodevelopfrom
jafranc/feat/localNewell

Conversation

@jafranc
Copy link
Copy Markdown
Contributor

@jafranc jafranc commented Feb 20, 2026

This PR draft a proposal for switching from Newell's normal method based on global coordinates to local, i.e. from
\mathbf{n} = \sum_i P_i x P_(i+1)%n to \mathbf{n} = \sum_i (P_i - P_0) x (P_(i+1)%n - P_0).

This could avoid some precision loss when dealing with large coordinates.

Tests are also more complete.

@jafranc jafranc self-assigned this Feb 20, 2026
@jafranc jafranc marked this pull request as ready for review February 26, 2026 19:58
@jafranc jafranc requested a review from castelletto1 February 26, 2026 19:59
@jafranc jafranc changed the title Feat: Change to local Newell's normal feat: Change to local Newell's normal Feb 26, 2026
Copy link
Copy Markdown
Contributor

@dkachuma dkachuma left a comment

Choose a reason for hiding this comment

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

Looks good to me and indeed should have been included in the original implementation. 2280

@castelletto1 castelletto1 added type: feature New feature or request ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI ci: run code coverage enables running of the code coverage CI jobs labels Apr 7, 2026
@castelletto1 castelletto1 requested a review from bd713 as a code owner April 7, 2026 17:21
@castelletto1 castelletto1 added the flag: requires rebaseline Requires rebaseline branch in integratedTests label Apr 9, 2026
@castelletto1
Copy link
Copy Markdown
Collaborator

@jafranc Please confirm that the failing integrated tests are behaving as expected, and verify the new baselines so we can proceed with merging.

@jafranc
Copy link
Copy Markdown
Contributor Author

jafranc commented Apr 10, 2026

@jafranc Please confirm that the failing integrated tests are behaving as expected, and verify the new baselines so we can proceed with merging.

I am looking into that

@jafranc
Copy link
Copy Markdown
Contributor Author

jafranc commented Apr 14, 2026

@jafranc Please confirm that the failing integrated tests are behaving as expected, and verify the new baselines so we can proceed with merging.

I am looking into that

Most of the failed test are exhibiting 1e-9 diff, which I would expect but the deadOilEgg:

********************************************************************************
Error: /Problem/Solvers/compositionalMultiphaseWell/wellControls7/dCurrentBHP
	Arrays of types float64 and float64 have 4 values of which 1 fail both the relative and absolute tests.
		Max absolute difference is at index (np.int64(2),): value = -98.10000000000002, base_value = -0.0
		Max relative difference is at index (np.int64(2),): value = -98.10000000000002, base_value = -0.0
	Statistics of the q values greater than 1.0 defined by absolute tolerance: N = 1
		max = 98100000.00000003, mean = 24525000.000000007, std = 42478546.05562673
		max is at index (np.int64(2),), value = -98.10000000000002, base_value = -0.0
	Statistics of the q values greater than 1.0 defined by relative tolerance: N = 0
********************************************************************************
********************************************************************************
Error: /Problem/Solvers/compositionalMultiphaseWell/wellControls7/dCurrentTotalVolumetricRate
	Arrays of types float64 and float64 have 5 values of which 1 fail both the relative and absolute tests.
		Max absolute difference is at index (np.int64(1),): value = 1.6560123574322675e-06, base_value = -7.86199250195236e-06
		Max relative difference is at index (np.int64(1),): value = 1.6560123574322675e-06, base_value = -7.86199250195236e-06
	Statistics of the q values greater than 1.0 defined by absolute tolerance: N = 1
		max = 9.518004859384629, mean = 1.9036009718769258, std = 3.8072019437538516
		max is at index (np.int64(1),), value = 1.6560123574322675e-06, base_value = -7.86199250195236e-06
	Statistics of the q values greater than 1.0 defined by relative tolerance: N = 0
********************************************************************************
********************************************************************************

strangly enough when compared proposed and develop normals:

  normal ( 0 1 0)                                                                   | normal ( 0 1 -5.78069e-16)                                                        
 normal ( -1 0 0)                                                                  | normal ( -1 0 0)                                                                  
 normal ( 0 0 -1)                                                                  | normal ( 0 0 -1)                                                                  
 normal ( 0 1 0)                                                                   | normal ( 0 1 -1.90883e-15)                                                        
 normal ( 1 0 0)                                                                   | normal ( 1 0 -2.40862e-15)                                                        
 normal ( 0 0 -1)                                                                  | normal ( 0 0 -1)                                                                  
 normal ( 0 1 0)                                                                   | normal ( 0 1 -2.75787e-15)                                                        
 normal ( 1 0 0)                                                                   | normal ( 1 0 0)                                                                   
 normal ( 0 0 -1)                                                                  | normal ( 0 0 -1)                                                                  
 normal ( 0 1 0)                                                                   | normal ( 0 1 2.6555e-15)                                                          
 normal ( 1 0 0)                                                                   | normal ( 1 0 -4.81724e-15)                                                        
 normal ( 0 0 -1)                                                                  | normal ( 0 0 -1)                                                                  
 normal ( 0 1 0)                                                                   | normal ( 0 1 6.6237e-15)                                                          
 normal ( 1 0 0)                                                                   | normal ( 1 0 9.63448e-15)                                                         
 normal ( 0 0 -1)                                                                  | normal ( 0 0 -1)                                                                  
 normal ( 0 1 0)                                                                   | normal ( 0 1 9.38759e-15)                                                         
 normal ( 1 0 0)                                                                   | normal ( 1 0 9.63448e-15)   

so I am looking into upwinding or a normal conditions that could flip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run code coverage enables running of the code coverage CI jobs ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: requires rebaseline Requires rebaseline branch in integratedTests type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants