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.

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

Add optimized linear system assembly for finite volume computations #25745

Merged
merged 142 commits into from
Mar 27, 2024

Conversation

grmnptr
Copy link
Contributor

@grmnptr grmnptr commented Oct 13, 2023

Refs #25722

Major issues to fix:

  • Cleaning up MooseVariable inheritance/interfaces for linear variables.
  • Improve test coverage.
  • Unify solve systems

@grmnptr grmnptr force-pushed the linear-system-25722 branch 2 times, most recently from 1cd22b0 to 0405e4b Compare October 19, 2023 01:29
@grmnptr grmnptr force-pushed the linear-system-25722 branch 2 times, most recently from 743e1a9 to a498165 Compare November 8, 2023 21:10
@grmnptr grmnptr changed the title [WIP] Add optimized linear system assembly Add optimized linear system assembly Jan 26, 2024
@idaholab idaholab deleted a comment from moosebuild Jan 26, 2024
@moosebuild
Copy link
Contributor

moosebuild commented Jan 26, 2024

Job Documentation on adbef30 wanted to post the following:

View the site here

This comment will be updated on new commits.

@grmnptr grmnptr force-pushed the linear-system-25722 branch 5 times, most recently from f815da4 to 6c962f1 Compare January 29, 2024 22:49
@moosebuild
Copy link
Contributor

moosebuild commented Jan 30, 2024

Job Coverage on adbef30 wanted to post the following:

Framework coverage

e235a2 #25745 adbef3
Total Total +/- New
Rate 85.29% 85.14% -0.15% 77.87%
Hits 101437 102535 +1098 1626
Misses 17500 17902 +402 462

Diff coverage report

Full coverage report

Modules coverage

Heat transfer

e235a2 #25745 adbef3
Total Total +/- New
Rate 88.46% 88.46% - 100.00%
Hits 4055 4055 - 1
Misses 529 529 - 0

Diff coverage report

Full coverage report

Navier stokes

e235a2 #25745 adbef3
Total Total +/- New
Rate 84.78% 84.78% - 100.00%
Hits 14596 14596 - 1
Misses 2620 2620 - 0

Diff coverage report

Full coverage report

Solid mechanics

e235a2 #25745 adbef3
Total Total +/- New
Rate 84.78% 84.78% - 100.00%
Hits 27109 27109 - 2
Misses 4868 4868 - 0

Diff coverage report

Full coverage report

Thermal hydraulics

e235a2 #25745 adbef3
Total Total +/- New
Rate 89.11% 89.10% -0.00% 100.00%
Hits 13970 13968 -2 2
Misses 1708 1708 - 0

Diff coverage report

Full coverage report

Full coverage reports

Reports

Warnings

  • framework new line coverage rate 77.87% is less than the suggested 90.0%

This comment will be updated on new commits.

@idaholab idaholab deleted a comment from moosebuild Jan 30, 2024
@grmnptr grmnptr marked this pull request as ready for review January 30, 2024 20:03
@grmnptr grmnptr changed the title Add optimized linear system assembly Add optimized linear system assembly for finite volume computations Jan 30, 2024
Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

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

Just through the first two files. Very nice design page. I will continue reviewing tomorrow

framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
framework/doc/content/finite_volumes/linear_fv_design.md Outdated Show resolved Hide resolved
Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

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

beginning to glance through the code, I haven't run into big concerns yet. A couple thoughts I do have:

  • There seems like a fair amount of dead, unused code in LinearSystem. Can we remove some of it or test some of it or both?
  • I noticed that the majority of uncovered code is from overrides of methods in MooseVariableField that simply aren't relevant to your new variable. Would it make sense to create another class, perhaps MooseVariableNonlinearField or something like that, that sits in between MooseVariableField and MooseVariableFE/FV that has all those pure virtuals that are irrelevant to your class?

framework/include/systems/LinearSystem.h Outdated Show resolved Hide resolved
framework/include/systems/LinearSystem.h Outdated Show resolved Hide resolved
@lindsayad
Copy link
Member

  • Would it make sense to create another class, perhaps MooseVariableNonlinearField or something like that, that sits in between MooseVariableField and MooseVariableFE/FV that has all those pure virtuals that are irrelevant to your class?

Another idea would be to inherit directly from MooseVariableFieldBase. I haven't investigated either idea thoroughly. I know you rely on functors, but you also don't support automatic differentiation, so perhaps inheriting from FunctorBase<Real> would actually be a good thing

@grmnptr
Copy link
Contributor Author

grmnptr commented Feb 1, 2024

Well, I would like to keep using variable-based interfaces (throught the FE problem and variablke interface) but I will definitely think about how to make this part cleaner! Yeah FunctorBase<ADReal> right now is not ideal. Would it add additional costs if the derivative containers are always empty?

@lindsayad
Copy link
Member

Well, I would like to keep using variable-based interfaces (throught the FE problem and variablke interface) but I will definitely think about how to make this part cleaner!

You may need to make new interfaces. But I think a design in which we don't have all these erroring overrides would lead to a lot of good cleanup. It would likely make it impossible to use a linear variable in a place where only a nonlinear variable will work and visa versa

grmnptr and others added 27 commits March 26, 2024 11:58
This member can be used in downstream objects which use functor
interfaces and hence don't care why type of field variable the
variable is
@grmnptr grmnptr merged commit 80c0cd2 into idaholab:next Mar 27, 2024
60 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants