Skip to content

Conversation

MarcelKoch
Copy link
Collaborator

Motivation

Replaces the Ginkgo solvers, by a single solver which is configured through a ginkgo property tree.

This allows to use any Ginkgo solver and preconditioner combination that is available.

TODO: check how the value type in the property tree should be handled.

@MarcelKoch MarcelKoch self-assigned this Mar 5, 2025
Copy link

github-actions bot commented Mar 5, 2025

Deployed test documentation to https://exasim-project.com/NeoFOAM/Build_PR_272

Copy link
Collaborator

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

do you mean something like type_descriptor for valuetype?

Co-authored-by: Yu-Hsiang M. Tsai <yhmtsai@gmail.com>
@MarcelKoch MarcelKoch force-pushed the feat/ginkgo-config-solver branch from 398638c to 0c6cbe1 Compare March 6, 2025 08:23


NeoFOAM::la::ginkgo::BiCGStab<ValueType> solver(solution.exec(), fvSolution);
NeoFOAM::la::ginkgo::Solver<ValueType> solver(solution.exec(), fvSolution);
Copy link
Collaborator

Choose a reason for hiding this comment

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

line 116-119 format seems to be incorrect

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

clang format seems to be fine with it.

Comment on lines 32 to 39
if (auto node = parseAny(int {}))
{
return node;
}
if (auto node = parseAny((unsigned int) {}))
{
return node;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

any other integer format from Dictionary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Possibly, but it doesn't make much sense right now to check for every possibility, since the dictionary is too unconstrained.


#if NF_WITH_GINKGO

bool operator==(const gko::config::pnode& a, const gko::config::pnode& b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I create a pr in ginkgo-project/ginkgo#1801


template<typename ExecSpace>
bool isNotKokkosThreads(ExecSpace ex)
bool isNotKokkosThreads([[maybe_unused]] ExecSpace ex)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool isNotKokkosThreads([[maybe_unused]] ExecSpace ex)
bool isNotKokkosThreads(ExecSpace)

it should also eliminate the warning by deleting the variable name.
but I am okay with the keywords

- fixes tests
- fixes formatting

Co-authored-by: Yu-Hsiang M. Tsai <yhmtsai@gmail.com>
This is necessary to prevent clashes with existing cmake targets from third-party libraries build through CPM.
Copy link
Collaborator

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

I have created #274 for the supported type concern in Dictionary. Other that that, LGTM

exec, gko::dim<2> {nrows, nrows}, valuesView, colIdxView, rowPtrView
return gko::share(gko::matrix::Csr<ValueType, IndexType>::create(
exec,
gko::dim<2> {nrows, nrows},
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: nrows from mtx.nRows() not from rhs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For now the matrices are assumed to be square. I think this will also stay like this for quite a while.

@greole greole added the full-ci a label that triggers the full ci pipeline label Mar 17, 2025
@greole greole moved this to In Progress in 2nd Hackathon Mar 17, 2025
@greole greole merged commit 1462691 into stack/implicitOperators Mar 17, 2025
3 of 7 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2nd Hackathon Mar 17, 2025
@HenningScheufler HenningScheufler deleted the feat/ginkgo-config-solver branch May 3, 2025 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci a label that triggers the full ci pipeline

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants