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

Make dcdd shock capture optional for heat transfer #1268

Merged
merged 9 commits into from
Sep 7, 2024

Conversation

mivaia
Copy link
Collaborator

@mivaia mivaia commented Aug 28, 2024

Description

DCDD stabilization was always used for the heat equation. DCDD usage sometimes lead to results that were not expected. More investigation will be needed in the future to understand what is the effect of adding the DCDD stabilization.

DCDD is now disabled by default. A new assembler has been created in order to take it into account.

Testing

Modified existing tests using heat transfer.

Documentation

Added a set DCDD shock capture option in the multiphysics section.

Miscellaneous (will be removed when merged)

Checklist (will be removed when merged)

See this page for more information about the pull request process.

Code related list:

  • All in-code documentation related to this PR is up to date (Doxygen format)
  • Lethe documentation is up to date
  • The branch is rebased onto master
  • Code is indented with indent-all and .prm files (examples and tests) with prm-indent
  • If parameters are modified, the tests and the documentation of examples are up to date
  • Changelog (CHANGELOG.md) is up to date if the refactor affects the user experience or the codebase

Pull request related list:

  • No other PR is open related to this refactoring
  • Labels are applied
  • There are at least 2 reviewers (or 1 if small feature) excluding the responsible for the merge
  • If this PR closes an issue or is related to a project, it is linked in the "Projects" or "Development" section
  • If any future works is planed, an issue is opened
  • The PR description is cleaned and ready for merge

@mivaia mivaia requested a review from blaisb August 28, 2024 20:37
Copy link
Contributor

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

Small things to adress
Good work man. Thanks for this cool refactor
Cna you rebase also?

source/solvers/heat_transfer_assemblers.cc Outdated Show resolved Hide resolved
source/solvers/heat_transfer_assemblers.cc Outdated Show resolved Hide resolved
source/solvers/heat_transfer_assemblers.cc Outdated Show resolved Hide resolved
source/solvers/heat_transfer_assemblers.cc Outdated Show resolved Hide resolved
@mivaia mivaia force-pushed the make_dcdd_shock_capture_optional_for_heat_transfer branch from 75a1a3b to 4b9d295 Compare August 29, 2024 16:28
Copy link
Contributor

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

One last tinyc hange, then good to go

doc/source/parameters/cfd/multiphysics.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@voferreira voferreira left a comment

Choose a reason for hiding this comment

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

Just small comments. I will reread it if you change the parameter section. Otherwise, just pin me and I will approve it. Good job!

doc/source/parameters/cfd/multiphysics.rst Outdated Show resolved Hide resolved
doc/source/parameters/cfd/multiphysics.rst Outdated Show resolved Hide resolved
include/solvers/heat_transfer_assemblers.h Outdated Show resolved Hide resolved
source/core/parameters_multiphysics.cc Outdated Show resolved Hide resolved
@blaisb
Copy link
Contributor

blaisb commented Sep 7, 2024

@mivaia an advice please rebase master onto your branch instead of merging next time. We always wish to avoid merge commits.

@blaisb
Copy link
Contributor

blaisb commented Sep 7, 2024

Will merge after CI has passed

@blaisb blaisb merged commit b6090ce into master Sep 7, 2024
11 checks passed
@blaisb blaisb deleted the make_dcdd_shock_capture_optional_for_heat_transfer branch September 7, 2024 18:52
M-Badri pushed a commit to M-Badri/lethe that referenced this pull request Sep 29, 2024
Description
DCDD stabilization was always used for the heat equation. DCDD usage sometimes lead to results that were not expected. More investigation will be needed in the future to understand what is the effect of adding the DCDD stabilization.

DCDD is now disabled by default. A new assembler has been created in order to take it into account.

Testing
Modified existing tests using heat transfer.

Co-authored-by: Bruno Blais <[email protected]>
Former-commit-id: b6090ce
blaisb added a commit that referenced this pull request Sep 30, 2024
Description
DCDD stabilization was always used for the heat equation. DCDD usage sometimes lead to results that were not expected. More investigation will be needed in the future to understand what is the effect of adding the DCDD stabilization.

DCDD is now disabled by default. A new assembler has been created in order to take it into account.

Testing
Modified existing tests using heat transfer.

Co-authored-by: Bruno Blais <[email protected]>
Former-commit-id: b6090ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants