GEOS
GEOS copied to clipboard
GEOS-DEV
Hydrofracture solver performance degradation after PR #2623
Hello,
I noticed that the hydrofracture solver has become worse in performance after the commit 94e338c. The convergence speed is relatively slow. I am not sure which PR after commit 94e338c caused this issue.
I have compiled the branch develop latest and branch develop 94e338c based on ubuntu 22.04 and measured the time consumed by case kgdViscosityDominated_poroelastic_benchmark.xml as follows (export OMP_NUM_THREADS=1 mpirun -np 9 geos -i kgdViscosityDominated_poroelastic_benchmark.xml -x 3 -y 3 -z 1 ):
| kgdViscosityDominated_poroelastic_benchmark.xml | Branch develop latest elasped time |
Branch develop 94e338c elasped time |
|---|---|---|
| The first 1 second | 134 s | 97 s |
| The first 10 seconds | 1016 s | 620 s |
The results show that the develop 94e338c branch is faster than the develop latest by about 50%.
At first I suspected that commit 6d397f7 was the problem because it changed HydrofractureSolver.cpp a lot, but I tested it and found the performance degradation was not related to commit 6d397f7. I suspect that there might be some modifications after commit 94e338c and before commit 6d397f7 that affect the performance of the simulation.
I suggest that performance testing is necessary as well when submitting a new PR.
I have confirmed that the performance degradation is caused by PR #2623. It might be due to the inaccuracy of the stiffness matrix.
I have confirmed that the performance degradation is caused by PR #2623. It might be due to the inaccuracy of the stiffness matrix.
@karimifard do you have any insight?
Hello, @karimifard and myself have been testing the pknViscosityDominated_smoke.xml case, and we could see a big difference before and after this PR https://github.com/GEOS-DEV/GEOS/pull/2481, which quite changes the xml.
We tested with the same recent develop version of geos for both cases.
Can you also test on your sides and give us some feedback?
See also #2826
Appreciate. This issue will be closed once #2826 is addressed.
Hello, @karimifard and myself have been testing the
pknViscosityDominated_smoke.xmlcase, and we could see a big difference before and after this PR #2481, which quite changes thexml. We tested with the same recentdevelopversion ofgeosfor both cases.Can you also test on your sides and give us some feedback?
Of course. I will run some tests on pknViscosityDominated_smoke.xml using the latest version of the develop branch, both with and without #2481 applied. I will report the results and any issues I encounter as soon as possible.
Hello, @karimifard and myself have been testing the
pknViscosityDominated_smoke.xmlcase, and we could see a big difference before and after this PR #2481, which quite changes thexml. We tested with the same recentdevelopversion ofgeosfor both cases. Can you also test on your sides and give us some feedback?Of course. I will run some tests on
pknViscosityDominated_smoke.xmlusing the latest version of the develop branch, both with and without #2481 applied. I will report the results and any issues I encounter as soon as possible.
The log files with and without #2481 applied are: result_with2481.txt and result_without2481.txt
The convergence speed of two cases are quite different, but I think it due to the model (mesh) size and pump rate. The case without #2481 converges fast because the pump rate is relatively low for the larger model size.
Hi, Have you checked if kgdViscosityDominated_poroelastic_benchmark.xml is exactly the same for "develop" and "develop 94e338c" cases?
Thanks!
Hi, Have you checked if
kgdViscosityDominated_poroelastic_benchmark.xmlis exactly the same for "develop" and "develop 94e338c" cases? Thanks!
Yes, I actually compiled develop and develop 94e338c as two singularity image files and run the same kgdViscosityDominated_poroelastic_benchmark.xml file from the latest develop branch.
Hi, Have you checked if
kgdViscosityDominated_poroelastic_benchmark.xmlis exactly the same for "develop" and "develop 94e338c" cases? Thanks!
@TotoGaz @karimifard Yes, there are some changes for the smoke tests of hydrofrac examples, which is necessary for the newly added curve checks in the integratedTests. But all benchmark tests have not been updated since my last check (#2521), which show some performance issues recently.
@karimifard @TotoGaz any update?
So far I haven't find anything "wrong". At this point the performance difference seems to be due to nonlinearity treatment. Essentially the update of the aperture in the flow discretization is lagging and the Jacobian accounts only for the impact of the aperture on permeability variation. I find the difference in performance to be too important, but maybe it's normal, I don't really know. It's possible that the hydrofrac model amplifies these effects. There is another thing I want to check is to see if there is something in hydrofrac algo which may not be compatible with the way we are updating the discretization...
@karimifard @TotoGaz any update?
So far I haven't find anything "wrong". At this point the performance difference seems to be due to nonlinearity treatment. Essentially the update of the aperture in the flow discretization is lagging and the Jacobian accounts only for the impact of the aperture on permeability variation. I find the difference in performance to be too important, but maybe it's normal, I don't really know. It's possible that the hydrofrac model amplifies these effects. There is another thing I want to check is to see if there is something in hydrofrac algo which may not be compatible with the way we are updating the discretization...
@karimifard Thanks for your work. Could you please consider reverting it for now until the performance issue is resolved?
Hi @paveltomin @TotoGaz @jhuang2601 @karimifard , I have resolved the performance issue as described in #2922 . I kindly requests you to check the PR and provide your feedback when you have time.