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

fix elastic seismos bug #2849

Merged
merged 17 commits into from
Dec 14, 2023
Merged

fix elastic seismos bug #2849

merged 17 commits into from
Dec 14, 2023

Conversation

tbeltzun
Copy link
Contributor

@tbeltzun tbeltzun commented Nov 27, 2023

The rewrite of #2729 introduced a bug in the elastic solver, where the derived class had members shadowing the parent class variables.

The cleanup was done in #2557 but was somehow missed when cherry-picking commits to create #2729.

This PR:

  • fixes elastic solver bug (member variables shadowing), reported by @shenchengyi, reproducer using inputFiles/wavePropagation/elas3D_DAS_smoke.xml: empty seismos time and sampled value;
  • make initTrace consistent with writeSeismoTrace i.e. don't touch the file if outputSeismoTrace is unset;
  • remove redundant / unused ElasticWaveEquationSEM::getNumNodesPerElem;
  • code style consistency across wavePropagation solvers files (e.g. use e for element);
  • use pow(..., n) patterns for clarity.

Rebaseline GEOS-DEV/integratedTests#71.

@tbeltzun tbeltzun added type: bug Something isn't working type: cleanup / refactor Non-functional change (NFC) labels Nov 27, 2023
@tbeltzun tbeltzun self-assigned this Nov 27, 2023
@tbeltzun tbeltzun added the flag: requires rebaseline Requires rebaseline branch in integratedTests label Nov 27, 2023
@tbeltzun tbeltzun requested review from sframba and acitrain November 27, 2023 12:24
@tbeltzun tbeltzun added the ci: run integrated tests Allows to run the integrated tests in GEOS CI label Nov 27, 2023
@tbeltzun tbeltzun added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Nov 28, 2023
@TotoGaz TotoGaz removed the ci: run integrated tests Allows to run the integrated tests in GEOS CI label Dec 13, 2023
@TotoGaz TotoGaz added the ci: run integrated tests Allows to run the integrated tests in GEOS CI label Dec 13, 2023
TotoGaz pushed a commit to GEOS-DEV/integratedTests that referenced this pull request Dec 14, 2023
@TotoGaz TotoGaz merged commit bbbab34 into develop Dec 14, 2023
20 checks passed
@TotoGaz TotoGaz deleted the bugfix/tbeltzun/seismos branch December 14, 2023 03:11
ouassimkh pushed a commit that referenced this pull request Feb 16, 2024
The rewrite of #2729 introduced a bug in the elastic solver, where the derived class had members shadowing the parent class variables.
The cleanup was done in #2557 but was somehow missed when cherry-picking commits to create #2729.
This PR:
- fixes elastic solver bug (member variables shadowing), reported by @shenchengyi, reproducer using `inputFiles/wavePropagation/elas3D_DAS_smoke.xml`: empty seismos time and sampled value;
- make `initTrace` consistent with `writeSeismoTrace` i.e. don't touch the file if `outputSeismoTrace` is unset;
- remove redundant / unused `ElasticWaveEquationSEM::getNumNodesPerElem`;
- code style consistency across `wavePropagation` solvers files (e.g. use `e` for element);
- use `pow(..., n)` patterns for clarity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: requires rebaseline Requires rebaseline branch in integratedTests type: bug Something isn't working type: cleanup / refactor Non-functional change (NFC)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants