-
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
Implementation of basic time integration #158
Conversation
Co-authored-by: bevanwsjones <bevanwsjones@gmail.com>
Co-authored-by: Henning Scheufler <henning.scheufler@web.de>
- updates docstrings - fix formating - fix some compilation issues
Implementation of operator coefficient handler
9cab253
to
a9bf558
Compare
Deployed test documentation to https://exasim-project.com/NeoFOAM/Build_PR_158 |
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.
I think we can put this in as in. Raised a few points but I think overall its good to go! 🥳
* | ||
* @return The size of the internal field | ||
*/ | ||
size_t size() const { return field_.internalField().size(); } |
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.
It might be better to in the future break this into a compund size return:
e.g.
std::vector<size_t> size_boundary(); // returns vector of every boundary size.
size_t size_internal(); // returns internal size.
vector<size_t> size(); // returns size of the internal field and then boundary field, by calling the above 2.
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.
or would it be an option to return a tuple of internal and boundary size?
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.
That is a better suggestion.
} | ||
|
||
// auto fB = Field(exec, 1, 4.0); | ||
// fvcc::TimeIntegration timeIntergrator(eqnSys, dict); |
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.
My only general comment is the test title and file are named 'time integration', but then we are testing via solve, which is a bit indirect. In any case for the moment, I don't think it matters - at least it's tested.
|
||
TimeIntegration(const Dictionary& dict) | ||
: timeIntegratorStrategy_( | ||
TimeIntegrationFactory<SolutionType>::create(dict.get<std::string>("type"), dict) |
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.
Does the create
function come from the RuntimeSelectionFactory
?
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.
yes!
|
||
static std::string schema() { return "none"; } | ||
|
||
virtual void solve(Expression& eqn, SolutionType& sol) const override |
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.
virtual void solve(Expression& eqn, SolutionType& sol) const override | |
void solve(Expression& eqn, SolutionType& sol) const override |
Co-authored-by: bevanwsjones <bevanwsjones@gmail.com>
b07b3fe
to
ce14d19
Compare
additional changes: