Skip to content

Fix segfault when an ACTIONX block drills a new well - #7258

Merged
akva2 merged 3 commits into
OPM:masterfrom
hnil:bugfix/actionx-new-well-parallel-info
Aug 19, 2026
Merged

akva2 merged 3 commits into
OPM:masterfrom
hnil:bugfix/actionx-new-well-parallel-info

Conversation

@hnil

@hnil hnil commented Jul 31, 2026

Copy link
Copy Markdown
Member

Two defects hit by actionx/ACTIONX_WELSPECS.DATA (WELSPECS inside an ACTIONX block):

  1. No ParallelWellInfo for the new well. The parallel well bookkeeping is built once from schedule().getWellsatEnd(), which does not contain action-created wells. Serial: createLocalParallelWellInfo dereferences end() → SIGSEGV in initializeWellPerfData (the guarding assert is compiled out in release). Parallel: not_on_process_ reports the well as remote on every rank, so it is silently dropped. Fixed by registering missing wells before the local well structure is built; entries are now held by pointer so adding one does not invalidate references handed out earlier.

  2. CPRW coarse system not resized. It reserves a row per well at creation; the well count then grows and the update path writes past the end. Fixed by rebuilding the solver when the well count changes.

Serial run now completes with the default solver; parallel reaches the existing distributed-well diagnostic and completes with --allow-distributed-wells=true.

Note: udq_actionx/ACTIONX_NE.DATA still fails, but for an unrelated reason — an OPEN COMPDAT connection in a cell that is not in the simulation grid, at report step 0.

Comment thread opm/simulators/wells/BlackoilWellModelGeneric.hpp Outdated
@hnil hnil added the manual:bugfix This PR is a bug fix and should be noted in the manual label Aug 2, 2026
@hnil
hnil marked this pull request as ready for review August 2, 2026 17:22
@hnil

hnil commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

The mutability is required: createLocalParallelWellInfo() hands the entries to initializeWellPerfData(), which calls beginReset(), pushBackEclIndex() and endReset() on them (BlackoilWellModelGeneric.cpp:446/483/504).

The const_cast was not, though — the elements are unique_ptrs and pointer constness does not carry to the pointee, so the non-const overload just does the lookup. Comment sharpened to say which calls need the handle.

@hnil
hnil force-pushed the bugfix/actionx-new-well-parallel-info branch from 323c280 to 8e1b6ea Compare August 7, 2026 14:46
@hnil

hnil commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

Is this ok now? @akva2

@akva2 akva2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks fine now. Could consider folding the call to registerNewParallelWells into getLocalWells - all existing getLocalWells call sites do both calls, I'm not sure if the convenience is worth it or not.

hnil added 3 commits August 11, 2026 14:06
The parallel well bookkeeping is built once, at construction, from
schedule().getWellsatEnd().  A well that WELSPECS creates inside an
ACTIONX block is not in that set, so it has no ParallelWellInfo entry:

  - serial: not_on_process_ short-circuits to false, the well enters
    wells_ecl_, and createLocalParallelWellInfo dereferences end() -
    SIGSEGV in initializeWellPerfData (the assert guarding this is
    compiled out in release builds);
  - parallel: not_on_process_ reports the well as remote on every rank,
    so it is silently dropped from the simulation.

Register the missing wells before the local well structure is built.
The schedule is replicated, so every rank visits the same new wells in
the same order - a requirement for the communicator split in
ParallelWellInfo's constructor.  Whether the new well perforates cells
on a rank has to be worked out from the connections, since a well
drilled by an action took no part in the grid partitioning.

Hold the entries by pointer so that adding one does not invalidate the
references handed out to the well container and the well state.

Repro: flow actionx/ACTIONX_WELSPECS.DATA.  In parallel the run now
reaches the existing distributed-well diagnostic and completes with
--allow-distributed-wells=true.
The CPRW coarse system reserves one row per well and is sized when the
preconditioner is created; the update path then writes a diagonal entry
per well into it.  A well drilled by an ACTIONX block raises the well
count mid-run, so an in-place update wrote past the end of the coarse
matrix.  There is an assert for exactly this, but it is compiled out in
release builds; what remains is a segfault in addWellPressureEquations.

Track the well count the solver was built for and force a full rebuild
when it changes.

With this, actionx/ACTIONX_WELSPECS.DATA runs to completion using the
default (CPR) solver, not just with --linear-solver=ilu0.
The lookup itself does not modify anything; only the path that hands the
information to initializeWellPerfData() and the well state needs a
mutable handle.
@hnil
hnil force-pushed the bugfix/actionx-new-well-parallel-info branch from 8e1b6ea to c869ed4 Compare August 11, 2026 12:06
@bska

bska commented Aug 11, 2026

Copy link
Copy Markdown
Member

jenkins build this please

@hnil

hnil commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Thanks. I left them separate: getLocalWells() is const and registerNewParallelWells() mutates, so folding would mean making the query non-const. That seemed a worse trade than the three call sites, but happy to change it if you disagree.

@akva2

akva2 commented Aug 19, 2026

Copy link
Copy Markdown
Member

makes sense.

@akva2
akva2 merged commit 68e3b68 into OPM:master Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:bugfix This PR is a bug fix and should be noted in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants