View Issue Details

IDProjectCategoryView StatusLast Update
0001824OpenFOAM[All Projects] Bugpublic2015-11-10 09:10
Reportermichael2015Assigned Tohenry 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformGNU/LinuxOSOtherOS Version2.3.0
Product Version 
Fixed in Version 
Summary0001824: solve ( ...) for a tmp<fvVectorMatrix> in case of a coupled matrix solver gives back an empty object of type solverPerformance
DescriptionDoes anybody know wy when I use a segregated approuch for the velocity solve ( ...) gives back a non empty opject of the type solverPerformance but when I solve it coupled solve (...) gives me back the standard constructor containing zeros for nItierations, initalResidual, finialResidual and so one.
TagsNo tags attached.

Activities

wyldckat

2015-10-18 18:30

updater  

fvMatrixSolve.C (9,806 bytes)

wyldckat

2015-10-18 18:30

updater  

SolverPerformance.C (6,577 bytes)

wyldckat

2015-10-18 18:30

updater  

SolverPerformance.H (8,252 bytes)

wyldckat

2015-10-18 18:45

updater   ~0005426

I was curious about this issue and I went looking into it. So the story seems to be something along these lines:

  The coupled solver was implemented just fine, but there was a limitation in the current framework that OpenFOAM uses for the matrix solvers: the solver performance can only report 1 initial residual and 1 final residual, but the coupled solver is able to report as many residuals as the components the equation has got.
  In the tutorial "incompressible/pisoFoam/ras/cavityCoupledU" test case, the solver performance is reported by the internal coupled solver as having 3 values for each residual value.

  The solution probably wasn't implemented back then, because a general implementation wasn't straight forward. Which is why the empty result was returned.


Either way, I was curious about this and I also needed to hone my skills in C++ templates, so attached is a proposition for this.


@Henry: Attached are the following 3 files:

 - fvMatrixSolve.C - for the folder "src/finiteVolume/fvMatrices/fvMatrix/" - it uses the new "summary" function that is defined in the other two files.

 - SolverPerformance.H and SolverPerformance.C - for the folder "src/OpenFOAM/matrices/LduMatrix/LduMatrix/" - the changes are as follows:

  - 2 new public methods named "maxInitialResidual()" and "maxFinalResidual()", which rely on the type defined in "pTraits<Type>::cmptType" for returning the maximum value from the respective arrays.

  - 1 new function named "summary" which is a convenience function for returning a summary instance of "SolverPerformance<scalar>", which technically is defined as "SolverPerformance<pTraits<Type>::cmptType>".

This isn't the most elegantly possible solution for this problem, but it's the best I could do for now.

henry

2015-10-18 19:02

manager   ~0005427

This is all pending the completion of the rewrite of the solvers in the new templated framework after which the residual information would be handled in a consistent manner. There are still some choices to be made, for example should the segregated approach be maintained? If so even with non-aligned symmetry planes, cyclics etc. for which the coupled approach would be better. There are many details still to be worked on for which significant funding will be required.

wyldckat

2015-10-18 19:19

updater   ~0005428

I expected as much, namely that this feature was only a precursor to something a lot greater.
But in the meantime, it's a bit sad that the returned value is an empty instance of "solverPerformance".

I didn't propose just one single function that did the summary, since it would look like a much bigger hack than the one I proposed... hence the two auxiliary methods. And it's consistent with how the results for the segregated solver are given for the U equation.

henry

2015-10-20 10:35

manager   ~0005443

I have reviewed your changes and I think this takes the code in the wrong direction. What is required is that fvMatrix<Type>::solve returns a SolverPerformance<Type> rather than a solverPerformance. This is more work but at least consistent with the next set of developments in this area of the code.

wyldckat

2015-10-20 10:44

updater   ~0005444

Last edited: 2015-10-20 10:47

View 2 revisions

Understood! The addition of the "max*Residual" methods are adding methods to the class that would only be temporary, and any other hack would just be a clunky way to postpone the problem. Therefore, if anything is done, might as well be in the right direction; and OpenFOAM-dev is for the next major version, which doesn't bother as much with retro-compatibility.

OK, then the previously attached code can at least help out those who are eager to use this feature.

I can look into using "SolverPerformance<Type>" in the upcoming weekend. It's a great opportunity to learn a few more details on some of the code's innermost workings.

henry

2015-10-20 11:52

manager   ~0005445

Agreed. For backward compatibility "SolverPerformance<Type>" can have a "max" method which returns a "SolverPerformance<scalar>" the equivalent of the current "solverPerformance" when returned from the segregated vector (or higher rank) solver.

wyldckat

2015-10-31 23:19

updater  

proposition_v2.tar.gz (16,237 bytes)

wyldckat

2015-10-31 23:33

updater   ~0005536

Attached is the tarball "proposition_v2.tar.gz" that has the following files (caution, it's meant to be unpacked within "OpenFOAM-dev"):

  applications/solvers/stressAnalysis/solidDisplacementFoam/solidDisplacementFoam.C
  applications/test/volField/Test-volField.C
  src/OpenFOAM/matrices/LduMatrix/LduMatrix/SolverPerformance.C
  src/OpenFOAM/matrices/LduMatrix/LduMatrix/SolverPerformance.H
  src/OpenFOAM/matrices/LduMatrix/LduMatrix/solverPerformance.C
  src/OpenFOAM/matrices/LduMatrix/LduMatrix/solverPerformance.H
  src/finiteVolume/fvMatrices/fvMatrix/fvMatrix.C
  src/finiteVolume/fvMatrices/fvMatrix/fvMatrix.H
  src/finiteVolume/fvMatrices/fvMatrix/fvMatrixSolve.C


Summary of proposed changes:

  1. solidDisplacementFoam.C: Adapted to using the new "max" function:

    - initialResidual = DEqn.solve().initialResidual();
    + initialResidual = max(DEqn.solve()).initialResidual();

  2. Test-volField.C: The solver performance is now saved to an object and outputted to the screen. In addition, an unbalanced "symmTensor" source was added to the equation, so that the residuals and solution was not identical for all of the solved components.

  3. SolverPerformance.[CH]: New friendly function "max" that returns the maximum instance as "SolverPerformance<typename Foam::pTraits<Type>::cmptType>"

  4. solverPerformance.[CH]: Specialization added for "SolverPerformance<scalar> max<scalar>(...)".

  5. fvMatrix.[CH]: All "solverPerformance" occurrences changed to "SolverPerformance<Type>".

  6. fvMatrixSolve.C:

     - Most "solverPerformance" occurrences changed to "SolverPerformance<Type>".

     - The method "solveSegregated" now uses a temporary and somewhat convoluted way of an instance of "SolverPerformance<Type>". It's the cleanest implementation I could figure out. You'll see what I mean after unpacking and seeing the changes.

     - The method "solveCoupled" now returns the true "SolverPerformance<Type>" result, instead of an empty instance.

     - The call to "setSolverPerformance" is now done by using the new "max" function, i.e. currently only storing the maximum solver performance indicators.


I only did a few tests, namely with "Test-volField" with the respective test case and the tutorial case "incompressible/icoFoam/cavity". Everything built successfully.


The next step forward would be to update "src/OpenFOAM/meshes/data/data.*" to the generic "SolverPerformance<Type>", but I haven't figured it out yet.

After that, if I saw correctly, then only "lduMatrix" is pending a revision.

wyldckat

2015-11-01 00:26

updater  

proposition_v3.tar.gz (3,756 bytes)

wyldckat

2015-11-01 00:36

updater   ~0005537

Attached is another tarball "proposition_v3.tar.gz", which will work on top of "proposition_v2.tar.gz".

The additional files provided in this tarball are as follows:

  src/OpenFOAM/meshes/data/data.C
  src/OpenFOAM/meshes/data/data.H
  src/OpenFOAM/meshes/data/dataTemplates.C
  src/finiteVolume/fvMatrices/fvMatrix/fvMatrixSolve.C

Summary of changes (continuing from the previous list):

  6.d. Removed the need to call "setSolverPerformance" with the "max" function.

  7. data.[CH]: The methods "setSolverPerformance" are now template based. The header now depends on the file "dataTemplates.C".

  8. dataTemplates.C: New file, which has the two template methods "setSolverPerformance".


If I'm not mistaken, this solves most of the issues for this report and pushes OpenFOAM towards the right evolutionary direction :)


The only thing that is bugging me is that the "singular_" variable in "SolverPerformance" isn't transferring properly between instances... should it be upgraded as well?

wyldckat

2015-11-02 21:49

updater   ~0005544

Yesterday I didn't find the time to do more tests, but today I've ran several tutorials using the script "Alltest" in the "tutorials" folder and it looks like the "proposition_v3" is still buggy :(
The error message:

        From function solution::solverDict(const word&)
        in file matrices/solution/solution.C at line 369
        Lookup solver for k
    smoothSolver: Solving for k, Initial residual = 1, Final residual = 0.00458206, No Iterations 1


    --> FOAM FATAL IO ERROR:
    wrong token type - expected Scalar, found on line 0 the punctuation token '('

    file: /home/ofuser/OpenFOAM/ofuser-dev/run/tutorialsTest/compressible/rhoPimpleFoam/ras/angledDuct/system/data.solverPerformance.U at line 0.

        From function operator>>(Istream&, Scalar&)
        in file lnInclude/Scalar.C at line 93.

    FOAM exiting


Looks like each major type will have to be stored in its own subdict block, to avoid these kinds of conflicts.

wyldckat

2015-11-08 20:44

updater  

proposition_v4.tar.gz (17,288 bytes)

wyldckat

2015-11-08 21:01

updater   ~0005586

OK, I think I got it down right this time. Attached is the file "proposition_v4.tar.gz" that provides all of the modified files and does not depend on the previous v2 and v3 attachments.

Again, the package is meant to be unpacked within "OpenFOAM-dev". The provided files in the package:

    applications/solvers/stressAnalysis/solidDisplacementFoam/solidDisplacementFoam.C
    applications/test/volField/Test-volField.C
    src/OpenFOAM/matrices/LduMatrix/LduMatrix/SolverPerformance.C
    src/OpenFOAM/matrices/LduMatrix/LduMatrix/SolverPerformance.H
    src/OpenFOAM/matrices/LduMatrix/LduMatrix/solverPerformance.C
    src/OpenFOAM/matrices/LduMatrix/LduMatrix/solverPerformance.H
    src/OpenFOAM/meshes/data/data.C
    src/OpenFOAM/meshes/data/data.H
    src/OpenFOAM/meshes/data/dataTemplates.C
    src/finiteVolume/fvMatrices/fvMatrix/fvMatrix.C
    src/finiteVolume/fvMatrices/fvMatrix/fvMatrix.H
    src/finiteVolume/fvMatrices/fvMatrix/fvMatrixSolve.C

Summary of proposed changes:

  1. solidDisplacementFoam.C: Adapted to using the new "max" function:

    - initialResidual = DEqn.solve().initialResidual();
    + initialResidual = DEqn.solve().max().initialResidual();

  2. Test-volField.C: The solver performance is now saved to an object and outputted to the screen. In addition, an unbalanced "symmTensor" source was added to the equation, so that the residuals and solution was not identical for all of the solved components.

  3. SolverPerformance.[CH]: 2 new methods:

    - "max()" that returns the maximum instance as "SolverPerformance<typename Foam::pTraits<Type>::cmptType>"

    - "replace(cmpt, sp)" is essentially the opposite of "max", which allows to replace a particular component by using a "SolverPerformance<typename Foam::pTraits<Type>::cmptType>".

  4. solverPerformance.[CH]: Specialization added for "SolverPerformance<scalar>::max()".

  5. fvMatrix.[CH]: All "solverPerformance" occurrences changed to "SolverPerformance<Type>".

  6. fvMatrixSolve.C:

     - Most "solverPerformance" occurrences changed to "SolverPerformance<Type>".

     - The critical part of the method "solveSegregated" was modified like this:

          - solverPerfVec = max(solverPerfVec, solverPerf);
          - solverPerfVec.solverName() = solverPerf.solverName();
          + solverPerfVec.replace(cmpt, solverPerf);

     - The method "solveCoupled" now returns the true "SolverPerformance<Type>" result, instead of an empty instance, and also stores the solver performance data into the dictionary.

  7. data.[CH]: A few modifications:

     - The methods "setSolverPerformance" are now template based. The header now depends on the file "dataTemplates.C".

     - And also added the method "solverPerformanceDict(const SolverPerformance<Type>&)" which returns a subdictionary into the main subdict, so that we store each type of solver performance inside a block for its own type; respective method specialization for "scalar" also added.

  8. dataTemplates.C: New file, which has the two template methods "setSolverPerformance" and the new template method "solverPerformanceDict(const SolverPerformance<Type>&)".


I've ran many of the tutorial cases by using "Alltest" in both OpenFOAM-dev (commit 2ac29110299, Oct 31) and OpenFOAM-2.4.x. There were no crashes like the ones I mentioned in the previous comment. And I didn't spot any major residual result discrepancies.

A few aesthetic details are probably missing, but I did the best I could do based on the instructions given at http://www.openfoam.org/contrib/code-style.php

henry

2015-11-09 09:26

manager   ~0005589

Excellent work Bruno. I am applying and testing these changes at the moment, if all goes well I should be able to push by the end of the day.

A couple of formatting tips:

    Set your editor to remove trailing whitespace automatically.
    This is trivial to setup in emacs and vim but I guess it can be done in all decent editors.

    Start comments with a space and capital letter i.e.
    // This....
    //not this...

One other point, could you update the copyright date range for file you change.

I have made the above changes to the files you provided so far so this is for future reference.

Thanks

henry

2015-11-10 09:10

manager   ~0005595

@Bruno: Thanks for the patches. In the form provided the residualControl and residual functionObject functionality no longer worked so I back-tracked on some of the changes and updated residualControl (backward-compatible) and the residual functionObject (upgraded to output the component residuals) and now all
the standard tests pass.

The changes are quite substantial and required a few core library changes to improve pTraits support so that all the types could be handled consistently including scalar without function specialization. You might find the way pTraits is used interesting.

Resolved in OpenFOAM-dev by commit 1944b09bb530a8b9a2546ac3b196b137668d329f

Issue History

Date Modified Username Field Change
2015-08-12 18:54 michael2015 New Issue
2015-10-18 18:30 wyldckat File Added: fvMatrixSolve.C
2015-10-18 18:30 wyldckat File Added: SolverPerformance.C
2015-10-18 18:30 wyldckat File Added: SolverPerformance.H
2015-10-18 18:45 wyldckat Note Added: 0005426
2015-10-18 18:46 wyldckat Assigned To => henry
2015-10-18 18:46 wyldckat Status new => assigned
2015-10-18 19:02 henry Note Added: 0005427
2015-10-18 19:19 wyldckat Note Added: 0005428
2015-10-20 10:35 henry Note Added: 0005443
2015-10-20 10:44 wyldckat Note Added: 0005444
2015-10-20 10:45 wyldckat Assigned To henry =>
2015-10-20 10:46 wyldckat Status assigned => new
2015-10-20 10:47 wyldckat Note Edited: 0005444 View Revisions
2015-10-20 11:52 henry Note Added: 0005445
2015-10-31 23:19 wyldckat File Added: proposition_v2.tar.gz
2015-10-31 23:33 wyldckat Note Added: 0005536
2015-11-01 00:26 wyldckat File Added: proposition_v3.tar.gz
2015-11-01 00:36 wyldckat Note Added: 0005537
2015-11-02 21:49 wyldckat Note Added: 0005544
2015-11-08 20:44 wyldckat File Added: proposition_v4.tar.gz
2015-11-08 21:01 wyldckat Note Added: 0005586
2015-11-08 21:01 wyldckat Assigned To => henry
2015-11-08 21:01 wyldckat Status new => assigned
2015-11-09 09:26 henry Note Added: 0005589
2015-11-10 09:10 henry Note Added: 0005595
2015-11-10 09:10 henry Status assigned => resolved
2015-11-10 09:10 henry Resolution open => fixed