Skip to content

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

Open
hnil wants to merge 3 commits into
OPM:masterfrom
hnil:bugfix/actionx-new-well-parallel-info
Open

Fix segfault when an ACTIONX block drills a new well#7258
hnil wants to merge 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.

hnil added 2 commits July 31, 2026 11:07
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.
/// \param wname Well name
/// \return Pointer to the well's information, or nullptr if the well is
/// not known to the parallel well bookkeeping.
ParallelWellInfo<Scalar>* findParallelWellInfo(const std::string& wname) const;

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.

mutability required?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You mean the return could be a const?

@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
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 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
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.

2 participants