fix: Bug with elastic wave propagator at higher orders #3294
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sframba
requested review from
acitrain,
rrsettgast,
castelletto1,
andrea-borio and
CusiniM
as code owners
sframba
added
type: bug
sframba
changed the title
acitrain
requested changes
Aug 21, 2024
src/coreComponents/finiteElement/elementFormulations/Qk_Hexahedron_Lagrange_GaussLobatto.hpp
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3294 +/- ##
===========================================
+ Coverage 56.56% 56.58% +0.01%
===========================================
Files 1064 1064
Lines 89689 89715 +26
===========================================
+ Hits 50737 50763 +26
Misses 38952 38952 ☔ View full report in Codecov by Sentry. |
sframba
added
flag: no rebaseline
sframba
changed the title
acitrain
approved these changes
Aug 23, 2024
|
@rrsettgast @castelletto1 @andrea-borio @CusiniM can any of you please give a quick review? This PR only impacts the wave propagators. Thanks! |
rrsettgast
approved these changes
Aug 27, 2024
|
@rrsettgast I think this PR is also fixing issue #3311, one of the main change inside this PR is the line you mentionned with the use of an other (and correct) elemToNodes map Update: I tried to run the test cases mentionned on the issue (which indeed crash on develop) using this branch and it don't throw any error, so I think that this PR indeed fix the issue |
rrsettgast
pushed a commit
that referenced
this pull request
Sep 17, 2024
* fixed a bug due to PR 3202 concerning the elastic solver at higher orders * added corresponding compute jacobian with corners * added correction for ghost nodes * Re-enable wave tests --------- Co-authored-by: acitrain <[email protected]>
rrsettgast
pushed a commit
that referenced
this pull request
Sep 17, 2024
* fixed a bug due to PR 3202 concerning the elastic solver at higher orders * added corresponding compute jacobian with corners * added correction for ghost nodes * Re-enable wave tests --------- Co-authored-by: acitrain <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR solves a bug introduced by PR #3202 in the case of the elastic solver at higher orders. The standard
calcGradNmethod of Gauss-Lobatto elements requires the coordinates of all the nodes in the mesh element. These nodes are no longer required for point location due to #3202, so only the mesh corners are passed toPrecomputeSourceAndReceiverTerm. However in the case of the elastic solver at higher orders, this interferes withcalcGradN.To correct this, a specialized version of the method,
calcGradNWithCorners, is introduced and used. This method only requires the mesh corners instead of all the element nodes, fixind the issue.resolves #3311