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

Multiple time integrators in an input #27687

Merged
merged 16 commits into from
Oct 30, 2024
Merged

Multiple time integrators in an input #27687

merged 16 commits into from
Oct 30, 2024

Conversation

lindsayad
Copy link
Member

@lindsayad lindsayad commented May 21, 2024 •
edited
Loading

Closes #19228

@lindsayad lindsayad changed the title Multiple ti 19228 [WIP] Multiple time integrators in an input May 21, 2024
@lindsayad lindsayad force-pushed the multiple-ti-19228 branch 3 times, most recently from 8272eed to 759ea9f Compare May 22, 2024 22:02
@lindsayad
Copy link
Member Author

relies on libMesh/libmesh#3856 and libMesh/libmesh#3857

@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity in the last 100 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale PRs that have reached or exceeded 90 days with no activity label Aug 31, 2024
@lindsayad
Copy link
Member Author

don't close

@github-actions github-actions bot removed the stale PRs that have reached or exceeded 90 days with no activity label Sep 7, 2024
@lindsayad lindsayad changed the title [WIP] Multiple time integrators in an input Multiple time integrators in an input Sep 24, 2024
@lindsayad lindsayad mentioned this pull request Sep 24, 2024
Open
@moosebuild
Copy link
Contributor

moosebuild commented Oct 11, 2024 •
edited
Loading

Job Documentation, step Docs: sync website on 69c996e wanted to post the following:

View the site here

This comment will be updated on new commits.

@lindsayad lindsayad changed the title Multiple time integrators in an input [WIP] Multiple time integrators in an input Oct 25, 2024
@lindsayad lindsayad changed the title [WIP] Multiple time integrators in an input Multiple time integrators in an input Oct 25, 2024
@moosebuild
Copy link
Contributor

moosebuild commented Oct 25, 2024 •
edited
Loading

Job Coverage, step Generate coverage on 69c996e wanted to post the following:

Framework coverage

f5a01d #27687 69c996
Total Total +/- New
Rate 85.06% 85.08% +0.01% 94.06%
Hits 106400 106569 +169 285
Misses 18682 18694 +12 18

Diff coverage report

Full coverage report

Modules coverage

Fsi

f5a01d #27687 69c996
Total Total +/- New
Rate 87.23% 87.23% - 100.00%
Hits 328 328 - 4
Misses 48 48 - 0

Diff coverage report

Full coverage report

Navier stokes

f5a01d #27687 69c996
Total Total +/- New
Rate 84.64% 84.64% - 100.00%
Hits 17081 17081 - 1
Misses 3100 3100 - 0

Diff coverage report

Full coverage report

Solid mechanics

f5a01d #27687 69c996
Total Total +/- New
Rate 84.88% 84.88% +0.00% 100.00%
Hits 28273 28274 +1 4
Misses 5038 5038 - 0

Diff coverage report

Full coverage report

Thermal hydraulics

f5a01d #27687 69c996
Total Total +/- New
Rate 88.34% 88.34% +0.00% 100.00%
Hits 13911 13913 +2 2
Misses 1836 1836 - 0

Diff coverage report

Full coverage report

Full coverage reports

Reports

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

Job OpenMPI on 8bb4b77 : invalidated by @lindsayad

@lindsayad lindsayad marked this pull request as ready for review October 28, 2024 00:34
loganharbour
loganharbour requested changes Oct 28, 2024
View reviewed changes
Copy link
Member

@loganharbour loganharbour left a comment

Choose a reason for hiding this comment

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

Looks great. Most of this is just suggestions

framework/include/systems/SystemBase.h Show resolved Hide resolved
framework/include/systems/SystemBase.h Outdated Show resolved Hide resolved
framework/include/systems/SystemBase.h Outdated Show resolved Hide resolved
framework/include/systems/SystemBase.h Show resolved Hide resolved
framework/include/timeintegrators/AStableDirk4.h Show resolved Hide resolved
framework/include/timeintegrators/TimeIntegrator.h Outdated Show resolved Hide resolved
framework/src/restart/DataIO.C Show resolved Hide resolved
framework/src/timeintegrators/AStableDirk4.C Outdated Show resolved Hide resolved
framework/src/timeintegrators/ImplicitEuler.C Show resolved Hide resolved
framework/src/timeintegrators/TimeIntegrator.C Outdated Show resolved Hide resolved
@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on b3dcdcb wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/27687/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format f5a01db01841e690890a2a245f55ea13fefb199f

@lindsayad @loganharbour
Apply suggestions from code review
69c996e 
- Remove commented method
- `getPossiblyNullTimeIntegrator` -> `queryTimeIntegrator`
- `const` addition to `from` vector in `copyVector`
- Add method name to `mooseError` in `TimeIntegrator::solve`
- Add comment regarding handling of null vectors
- Create computeDuDotDu and duDotDuCoeff methods

Co-authored-by: Logan Harbour <loganhharbour@gmail.com>
@lindsayad lindsayad merged commit b8db753 into idaholab:next Oct 30, 2024
51 checks passed
@lindsayad lindsayad deleted the multiple-ti-19228 branch October 30, 2024 04:15
@lindsayad
Copy link
Member Author

thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loganharbour loganharbour loganharbour approved these changes

@joshuahansel joshuahansel Awaiting requested review from joshuahansel joshuahansel is a code owner

@bwspenc bwspenc Awaiting requested review from bwspenc bwspenc is a code owner

@dschwen dschwen Awaiting requested review from dschwen dschwen is a code owner

@jiangwen84 jiangwen84 Awaiting requested review from jiangwen84 jiangwen84 is a code owner

@GiudGiud GiudGiud Awaiting requested review from GiudGiud GiudGiud is a code owner

@grmnptr grmnptr Awaiting requested review from grmnptr grmnptr is a code owner

@roystgnr roystgnr Awaiting requested review from roystgnr roystgnr is a code owner

Assignees

@lindsayad lindsayad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crank-Nicolson totally wrong if any PDE in the set missing a time derivative term
3 participants
@lindsayad @moosebuild @loganharbour