-
Notifications
You must be signed in to change notification settings - Fork 11
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.
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
Dsl/temporal operators #259
Dsl/temporal operators #259
Conversation
Deployed test documentation to https://exasim-project.com/NeoFOAM/Build_PR_259 |
@gregorweiss @greole @bevanwsjones @MarcelKoch I would propose to not add any more features to this PR and focus on improving code quality and merge it as quickly as possible in stack/implicitOperator. Currently, multiple PR adapt the DSL and introduce new features. If we quickly merge this PR, we can minimize the impact on the other PRs. The next Milestone would be to implement scalarAdvection with implicitOperators: With ideally required the following features:
|
NeoFOAM::dsl::TemporalOperator ddtOperator = NeoFOAM::dsl::imp::ddt(vf); | ||
|
||
// ddt(U) = f | ||
auto eqn = ddtOperator + dummy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto is a bit miss leading here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some quick comments
include/NeoFOAM/dsl/explicit.hpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove usage of namespace NeoFOAM:: inside the NeoFOAM namespace.
{ | ||
public: | ||
|
||
LinearSystem(VolumeField<ValueType>& psi) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use x
or solution
instead of psi
?
include/NeoFOAM/finiteVolume/cellCentred/operators/fvccLinearSystem.hpp
Outdated
Show resolved
Hide resolved
); | ||
} | ||
|
||
Field<IndexType> diagIndex() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is diagIndex? Please add a docstring
Co-authored-by: Gregor Olenik <gregor.olenik@web.de>
include/NeoFOAM/dsl/expression.hpp
Outdated
@@ -57,91 +52,80 @@ class Expression | |||
/* @brief perform all explicit operation and accumulate the result */ | |||
Field<scalar> explicitOperation(Field<scalar>& source) | |||
{ | |||
for (auto& oper : explicitOperators_) | |||
for (auto& oper : spatialOperators_) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using op
would be more consistent, cf with loop in L.44
for (auto& oper : spatialOperators_) | |
for (auto& op : spatialOperators_) |
} -> std::same_as<void>; // Adjust return type and arguments as needed | ||
}; | ||
|
||
/* @class Operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So most of the implementation is gone from here: can this file be removed all together otherwise maybe add some docstring why the Operator class is still needed and what the purpose is.
include/NeoFOAM/finiteVolume/cellCentred/operators/fvccLinearSystem.hpp
Outdated
Show resolved
Hide resolved
include/NeoFOAM/finiteVolume/cellCentred/operators/fvccLinearSystem.hpp
Outdated
Show resolved
Hide resolved
include/NeoFOAM/finiteVolume/cellCentred/operators/fvccLinearSystem.hpp
Outdated
Show resolved
Hide resolved
include/NeoFOAM/finiteVolume/cellCentred/operators/fvccLinearSystem.hpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Gregor Olenik <gregor.olenik@web.de>
7dc90b1
into
stack/implicitOperators
Motivation
adds implicit temporal operator
included features in NeoFOAM:
Advantages:
Cons:
included features in FoamAdapter:
ToDo: