Parallel improvement (switch back to 4-argument input).

[panasun1994]

The problem

Af ther I created the first documentation for running parallel from Python, I still have a question: What is the trade-off if you input only the file_name instead of the 4 args (dec, state, schedule, summary config)? The answer is that the simulator ignores the well control function, such as shut_well.

Why

Because when it comes to the mpirun from Python, in the readDeck function, if you have only file_name input, the simulator will create its own schedule on rank 0, and the schedule is empty on the other ranks. On the other hand, readDeck will keep the schedule in every rank when it receives 4 args as input. Without handling the schedule, you cannot control the well; i.e., you can't shut/open/close the well.

The solution

The real argument that prevents MPI parallelism in Python is the Eclipstate. So, we can normally input the 4args with state = None. The other 3 arguments will work as in the sequential case.

To Reviewer

You can write the Python code to test the shut_well function in both file_only input and 4 args input, and compare them. If you want my test code, please let me know. I am happy to share.

[hakonhagland]

@panasun1994 Thanks for this, and for tracking down the shut_well behavior. I have reproduced it: in a 4-rank run, a shut_well made through the Schedule passed to BlackOilSimulator(deck, None, schedule, summary_config) takes effect, while with the filename constructor there is no way to reach the simulator's own Schedule from Python at all.

Before I post a full review, I would like to check whether this is better fixed in the opm-simulators Python bindings themselves — for example, by letting the simulator return the Schedule it actually uses, so that well controls work with the filename constructor and the None workaround is not needed. If that turns out to be feasible, the documentation could stay simpler. I will get back to you here with the review either way.

[panasun1994]

@hakonhagland I agree with you. Once, we have to check on the C++ side whether we have to fix it on that end or not. I also have a plan to sit and look into the C++ side next week. But I think while we dig into it, the 4args Python script with state = None is the better temporary choice for the user.

[hakonhagland]

@panasun1994 Thanks again for this, and for your patience while I checked the bindings side.

The short answer from that check: the bindings can be fixed, but not quickly enough for this page to wait. A get_schedule() method that returns the Schedule the simulator actually uses is in OPM/opm-simulators#7500 (still a draft, waiting for #7444). With it, sim.get_schedule().shut_well(...) works with the filename constructor on 4 ranks, with the same result as your None form. But @bska has pointed out that OPM/opm-common#5371, a rework of the Schedule object, will replace the step-indexed calls such as shut_well(name, step) themselves, and that will not be ready before the end of October. So I agree with you: for now, the four-argument constructor with None is the way that works with released OPM, and it is worth documenting. Since you mentioned looking into the C++ side this week, please have a look at #7500 and opm-common#5371 first, so we do not end up doing the same work twice. Review comments or testing on #7500 would be very welcome.

Given that, I think the page should present the None form as what it is, a workaround until the simulator offers a supported way to reach its Schedule. With that framing I am happy to approve, once the three points below are addressed. The smaller items are inline.

1. The page contradicts itself. "Constructing the simulator" (lines 122–136, not part of this diff, so I cannot comment on it inline) still says "In parallel, only the filename constructor works" and that the four-argument form "cannot run on more than one rank", right below an example that now does exactly that. A suggested replacement for the warning:

.. warning::

   In parallel, the ``EclipseState`` argument of the four-argument
   constructor must be ``None``:

   .. code-block:: python

      sim = BlackOilSimulator(deck, None, schedule, summary_config)

   Passing an ``EclipseState`` object fails on more than one rank with
   ``Parallel simulator setup is incorrect as it does not use ParallelEclipseState``.
   With ``None``, the simulator builds the parallel state itself from the
   DATA file. The filename constructor, ``BlackOilSimulator(filename=...)``,
   also works in parallel, but then the script has no access to the
   ``Schedule`` the simulator uses, so well controls cannot be changed from
   Python.

2. The page does not say why the example changed. Well control is not mentioned anywhere on the page, so a reader sees a longer example and no reason for it. I have suggested a short paragraph above the example inline, and adding a shut_well call to the example itself, since that is the whole point of the change.

3. Every rank has to make the same well-control call. I measured this on 4 ranks with SPE1CASE1: schedule.shut_well("PROD", 3) called on all ranks shuts the well (its oil rate drops from 20000 to 0 for the remaining report steps), but the same call made only under if rank == 0: is silently ignored — no error, and the well keeps producing. Each rank holds its own Schedule, and each rank's simulator uses its own. Since the example already puts its final print under if RANK == 0:, this is an easy mistake to make, and the page should warn about it. Suggested wording is in the inline comment at step_init().

Optional, not worth another round on their own: the new code block is indented 4 spaces where the other blocks on the page use 3, which is valid but inconsistent.

[hakonhagland]

Thanks for working through these so quickly — the new paragraph explaining why the example uses the four-argument form reads well, and the restored comments help.

There is one new problem, and I think it came from applying the suggestions one at a time: the last commit (9fd911e) removed the if RANK == 0: print(...) and the if __name__ == "__main__": main() lines at the end of the script, so the example now defines main() and never calls it. Run as written, it exits immediately without simulating anything. My blank-line suggestion was meant for the three empty lines that used to be there, but by the time it was applied, the earlier suggestions had moved the lines down. Sorry about that — I have put a suggestion inline that restores the ending. Applying several suggestions as one batch ("Add suggestion to batch") avoids this kind of shift.

Two points from the first round are still open:

1. "Constructing the simulator" still contradicts the example (lines 132–146, outside this diff, so again in the review body). The new paragraph and the constructor comment both point the reader there for the explanation of None, and the section still says the four-argument form "cannot run on more than one rank". The suggested replacement from the first review, unchanged:

.. warning::

   In parallel, the ``EclipseState`` argument of the four-argument
   constructor must be ``None``:

   .. code-block:: python

      sim = BlackOilSimulator(deck, None, schedule, summary_config)

   Passing an ``EclipseState`` object fails on more than one rank with
   ``Parallel simulator setup is incorrect as it does not use ParallelEclipseState``.
   With ``None``, the simulator builds the parallel state itself from the
   DATA file. The filename constructor, ``BlackOilSimulator(filename=...)``,
   also works in parallel, but then the script has no access to the
   ``Schedule`` the simulator uses, so well controls cannot be changed from
   Python.

2. The page still does not say that every rank must make the well-control call. A shut_well made only on rank 0 is silently ignored, and the example's own if RANK == 0: makes that an easy mistake. I have put a one-sentence suggestion on the new paragraph inline, so it can be applied with one click.

With those three fixed, I am happy to approve.

[Copilot]

🟡 Changes recommended

The existing constructor warning contradicts the new example and must be updated.

1 open finding

• Update outdated parallel-constructor documentation · New

What changed in this PR

Updates parallel Python documentation to preserve schedule-based well control under MPI.

Changes:

• Uses the four-argument simulator constructor with state=None.
• Documents per-rank schedule updates and MPI setup.

File	Description
python/​sphinx_docs/​docs/​parallel-in-python.rst	Revises the parallel simulation example and constructor guidance.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[panasun1994]

@hakonhagland Thank you very much for the careful review. I accept most of the changes you suggested. According to the shut_well example, I added while not sim.check_simulation_finished(): over the sim.step() to collect the report. I also suggest that it would be nice to tell the user about the expected result and add the code to print it because the user may not know what to expect.

[panasun1994]

@hakonhagland Thanks a lot for opm-simulators/pull/7500. I am actually new here, and the Python binding codebase is huge. This PR help me a lot with scoping down.

[hakonhagland]

Thanks — this looks good now. The three open points are resolved, and I checked the result: the example runs as written on 4 ranks, and the well is shut as described. Adding the expected output is a good idea, I agree that readers will want to know what to look for. One small thing about it, inline: the output depends on which SPE1CASE1.DATA is used, and the page does not say. With the ten-day deck from the opm-simulators Python tests I get exactly the lists shown. With the ten-year deck from opm-tests, which the serial page recommends, the lists are 125 entries long and the well shows as shut from day 90 instead. I have suggested a sentence that names both. Please apply it before this is merged. Approving now so this does not need another round.

And thanks for the note about #7500 — glad it helped. Once that, or the Schedule rework in opm-common#5371, is available, this page can be simplified to use it.