OPM
Reservoir coupling: Implement time stepping
Builds on #5521 which should be merged first.
Also refactors the original adaptive time stepping method.
Builds on #5521 which should be merged first.
Okay, in that case I'm going to mark this as "work in progress/draft" to avoid inadvertent merging.
I'm going to mark this as "work in progress/draft" to avoid inadvertent merging.
@bska Thanks. I was actually planning to get this merged, since it is becoming difficult to handle all the dependent PRs and keep them up to date. Is this a good idea, or should I wait until the complete reservoir coupling feature is ready?
Is this a good idea, or should I wait until the complete reservoir coupling feature is ready?
It is a very good idea. Do not wait for the complete feature, unless you expect significant changes to the parts you already did. This patch has grown to become quite large, so finishing and polishing the first PRs in the series so they can be merged should be a high priority.
It is a very good idea.
@atgeirr Ok I agree. I have removed the draft state on https://github.com/OPM/opm-simulators/pull/5436 as it is the first in the dependency chain and we should try to merge that one first.
I think the jenkins failure is due to unconditional usage of MPI and unconditionally including mpi.h. Unfortunately, we require compiling without MPI to work. There is a subtle reason for this, but I fail to remember it.
Unfortunately, we require compiling without MPI to work.
@blattms How do I build locally without MPI? I tried cmake option -DUSE_MPI=NO but opm-grid fails to build with error /usr/bin/ld: cannot find -lMPI::MPI_C: No such file or directory
Unfortunately, we require compiling without MPI to work.
@blattms How do I build locally without MPI? I tried cmake option
-DUSE_MPI=NObut opm-grid fails to build with error/usr/bin/ld: cannot find -lMPI::MPI_C: No such file or directory
Use -DCMAKE_DISABLE_FIND_PACKAGE_MPI=ON, but you need to use the same options for all modules.
I was not able to rebase this on master (ref. private discussion with @blattms). (After resolving conflicts the resulting files were messed up and couldn't even be compiled). As a workaround I am merging master into this branch instead. The latest merge commit incorporates the following pull requests that have been merged into master since last time master was merged and which also changed the file AdaptiveTimeStepping.hpp:
- #2970: remove 16 days restriction for timestep in predicition mode
- #5803: remove unused fipnum argument
- #5792: added: support WCYCLE
As a workaround I am merging master into this branch instead
In my experience that creates an almost impossible mess to extract oneself from. While I do not know what was the reason for the rebasing failures, it can sometimes help to squash the commits on your branch (into a single commit) and then rebase, at least then each conflict will only arise once whereas otherwise you may have a conflict that appears over and over again every time your branch commits touch the same code parts. Maybe you could try again after first squishing?
Advice from git experts welcome!
This cannot be rebased on master since it's based on the spawn_slaves branch and for some reason the contents of that branch is not here. Rebasing spawn_slaves on master, then this PR on spawn slaves, all seems fine to me, as in it builds fine. I haven't tested runtime.
I would start by extracting some refactorizations from this branch. Splitting AdaptiveTimeStepping.hpp into an impl file is bound to create conflicts if such a change lives long. This can be done completely separately from the other work in this PR, and will make rebasing this so much easier on you.
since it's based on the spawn_slaves branch
@akva2 How did you check that? I tried:
$ git branch --show-current
timestepping2
$ git merge-base --is-ancestor spawn_slaves HEAD || echo "Not based on spwan_slaves"
Not based on spwan_slaves
so it seems it is not based on that branch?
First commit in this branch modifies ReservoirCouplingMaster.cpp and that file looked to me as is added in https://github.com/hakonhagland/opm-simulators-1/commit/13c2b43ce4ffa1e89ace3e83cb965c578e708b20
And yes, the tool says exactly the same I said - this branch does not contain spawn_slaves but it should, or at least parts of it, as the first commit tries to change a file added in that branch.
and that file looked to me as is added in https://github.com/hakonhagland/opm-simulators-1/commit/13c2b43ce4ffa1e89ace3e83cb965c578e708b20
The commit message of that commit should be "Spawn slaves from master", right? However, I cannot find that commit:
$ git branch --show-current
timestepping2
$ git log --oneline --grep="Spawn slaves from master"
# No output
Yes. You are going about this the wrong way around.
I told you what the PROBLEM is. The problem is that commit IS NOT in this branch.
Hence the first commit in this branch, namely https://github.com/OPM/opm-simulators/pull/5620/commits/b40f5ec9a2eedbe96876097fd10e36681c455e28 cannot apply because the file it is trying to patch does not exist.