Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Determine solver type by pressure dimension #124

Merged
merged 9 commits into from
Mar 20, 2020
Merged

Determine solver type by pressure dimension #124

merged 9 commits into from
Mar 20, 2020

Conversation

davidscn
Copy link
Member

This PR determines the solver type via the pressure dimension.

Here a few comments, following from our discussion in #31:

The type 'basic' is assumed instead of naming it 'other'. So we raise a warning and tell the user, that he might get issues with this settings. I preferred now 'basic', since this is an actual OpenFOAM type and the user might (or not) be fine with the settings. If we tell the user, that we calculate with type 'other', no one knows what this means without looking in the code.

I added the checker in the module classes and decided not to pass it in the adapter class, because we first read the configuration in the specific module and start afterwards to determine the type 'automatically' and we cannot access (without passing the whole class) the adapter class in the modules.
EDIT: We could determine it in the adapter class and overwrite it in the modules.

However, there are just some conceptual questions. The general ingredients should be fine.

Closes #31.

@davidscn davidscn requested a review from MakisH March 15, 2020 08:37
@davidscn davidscn changed the base branch from master to develop March 15, 2020 08:39
@davidscn
Copy link
Member Author

davidscn commented Mar 15, 2020 •
edited
Loading

Sorry, my base branch is now OF 6. The committed changes are the same. I (or whoever) need to consider this during merging.

MakisH
MakisH requested changes Mar 16, 2020
View reviewed changes
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

The mechanism looks good! Let's decide on the default solver name (see comment) and then we can try it like this in real scenarios.

Afterwards, we could also try moving it into Adapter, but now the respective code is much much shorter that I am not sure if it would solve more problems than it would create. A user-defined module may implement additional types (e.g. linear model vs non-linear model for XYZ) and may want to combine their module with an existing one.

@davidscn
Copy link
Member Author

Afterwards, we could also try moving it into Adapter, but now the respective code is much much shorter that I am not sure if it would solve more problems than it would create. A user-defined module may implement additional types (e.g. linear model vs non-linear model for XYZ) and may want to combine their module with an existing one.

Yes! I think propagating it in the end to the proper place will not save any line of code.

@davidscn davidscn force-pushed the solver_type_via_pressure branch from 80aa9f2 to 0aa8823 Compare March 18, 2020 10:29
@davidscn
Copy link
Member Author

TODO: Update FSI.C

@davidscn davidscn changed the title WIP: Determine solver type by pressure dimension Determine solver type by pressure dimension Mar 20, 2020
MakisH
MakisH reviewed Mar 20, 2020
View reviewed changes
MakisH
MakisH approved these changes Mar 20, 2020
View reviewed changes
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

Looks good! I would also add these small changes. Other than that, please test it with the solvers we use in our tutorials (pimpleFoam, buoyantPimpleFoam, buoyantSimpleFoam, laplacianFoam) and feel free to merge.

davidscn and others added 4 commits March 20, 2020 12:05
@davidscn @MakisH
Update FSI/FSI.C
aa661ac 
Co-Authored-By: Gerasimos Chourdakis <makishourdakis@gmail.com>
@davidscn @MakisH
Update CHT/CHT.C
4f91a58 
Co-Authored-By: Gerasimos Chourdakis <makishourdakis@gmail.com>
@davidscn
Remove basic option
55e0d98
@davidscn
Improve debug messages
f8c0d9d
@davidscn
Copy link
Member Author

Tested and works as expected!

@davidscn davidscn merged commit b7c7a7c into develop Mar 20, 2020
@davidscn davidscn deleted the solver_type_via_pressure branch March 20, 2020 19:52
@MakisH MakisH mentioned this pull request Mar 23, 2020
Closed
@MakisH MakisH mentioned this pull request Jul 22, 2020
Merged
pachesp added a commit that referenced this pull request Sep 30, 2020
@pachesp
Squashed commit of the following:
2a996f2 
commit 045d0d2
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Wed Jul 29 13:54:01 2020 +0200

    Cleanup sonicLiquidFoam case files

commit 91160d7
Author: uekerman <benjamin.uekermann@gmail.com>
Date:   Wed Jul 29 10:13:52 2020 +0200

    Tune FF sonicLiquidFoam to converge

commit c0e61f5
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Fri Jul 24 14:04:39 2020 +0200

    Add todo for determining solverType of multiphase solvers

commit 35cc2c2
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Wed Jul 22 21:57:15 2020 +0200

    Add pressure-based solver type guessing

commit c38f109
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Wed Jul 22 21:41:43 2020 +0200

    Update FF/FF.C

commit 03cee93
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Wed Jul 22 21:38:32 2020 +0200

    Update preCICE logger for consistency

commit fcb2c37
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Wed Jul 22 21:35:14 2020 +0200

    Remove density from transportProperties

commit a39035d
Merge: 78fc865 46c1dcf
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Wed Jul 22 18:22:28 2020 +0200

    Merge branch 'develop' into FF

commit 46c1dcf
Merge: 84a3178 0eb0742
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Tue Jun 30 10:47:18 2020 +0200

    Merge pull request #131 from precice/refactor-assign-construct

    Refactor to reduce assigning fields at creation

commit 0eb0742
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Mon Jun 29 21:21:45 2020 +0200

    Refactor to reduce assigning fields at creation

    This replaces statements such as:

        scalarField one_field = another_field;

    by constructors:

        scalarField one_field(another_field);

    This can be safer and faster, as it avoids implicit conversions in between.

commit 84a3178
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Tue Jun 23 21:19:26 2020 +0200

    Tutorials: Add removeObsoleteFolders.sh script

    Related to #26

    Already implemented separately for the FSI tutorials
    in the precice/tutorials repository.

commit 55a1d57
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Tue Jun 23 21:03:51 2020 +0200

    Simplify precice-config.xml of tutorials

    Hides the timing:initial of mapping (not needed) and extrapolation-order (not important).
    Moves the max-iterations together with the relative-convergence-measure (more natural to explain).

commit 671104a
Author: David Schneider <dav.schneider@tum.de>
Date:   Mon Jun 8 10:27:37 2020 +0200

    Support stress data (#125)

    Add support for stress data writing

commit 42a3d6d
Author: Gerasimos Chourdakis <makishourdakis@gmail.com>
Date:   Tue Mar 24 10:07:35 2020 +0100

    Workaround for #109 (NP tutorial)

commit b7c7a7c
Author: David Schneider <dav.schneider@tum.de>
Date:   Fri Mar 20 20:39:04 2020 +0100

    Determine solver type by pressure dimension (#124)

    Determine solver type by pressure dimensions and throw error if automatic process fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MakisH MakisH MakisH approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Check for (in)compressible solver type with Pressure dimensions
2 participants
@davidscn @MakisH