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

rewrite seismo traces - rework WaveSolverBase structure #2729

Merged
merged 18 commits into from
Nov 9, 2023

Conversation

tbeltzun
Copy link
Contributor

@tbeltzun tbeltzun commented Sep 29, 2023

  • move computeAllSeismoTraces and compute2dVariableAllSeismoTraces to WaveSolverBase;
  • simplify WaveSolverBase and move members (such as m_rcvElem or m_receiverRegion) / keys from children solvers to parent class;
  • rewrite computeAllSeismoTraces for loops for clarity;
  • rewrite writeSeismoTrace;
  • add writeSeismoTraceVector helper;
  • add initTrace to avoid appending to an initial seismo trace;
  • fix a subtle domain decomposition bug where a receiver is not owned by a single MPI process (early abort instead);
  • enhance seismotrace sampling by adding time, enhance robustness.

Rebaseline PR: GEOS-DEV/integratedTests#48.

Moved out of #2557.

@tbeltzun tbeltzun added type: bug Something isn't working type: feature New feature or request ci: run CUDA builds Allows to triggers (costly) CUDA jobs flag: ready for review labels Sep 29, 2023
@tbeltzun tbeltzun self-assigned this Sep 29, 2023
@tbeltzun tbeltzun requested review from acitrain and sframba October 6, 2023 09:01
@tbeltzun tbeltzun removed the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Oct 6, 2023
@tbeltzun tbeltzun force-pushed the bugfix/tbeltzun/seismo-rewrite branch from 1bf2359 to bb54aed Compare October 12, 2023 15:01
@tbeltzun
Copy link
Contributor Author

@acitrain, @sframba, I've cleaned the PR and adjusted it for #2544 so I think it can be reviewed now.

@tbeltzun tbeltzun added ci: run CUDA builds Allows to triggers (costly) CUDA jobs type: cleanup / refactor Non-functional change (NFC) flag: requires rebaseline Requires rebaseline branch in integratedTests and removed flag: ready for review labels Oct 13, 2023
@castelletto1 castelletto1 merged commit 625714b into develop Nov 9, 2023
18 checks passed
@castelletto1 castelletto1 deleted the bugfix/tbeltzun/seismo-rewrite branch November 9, 2023 03:14
TotoGaz pushed a commit that referenced this pull request Dec 14, 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.
ouassimkh pushed a commit that referenced this pull request Feb 16, 2024
* rewrite seismo traces - rework `WaveSolverBase` structure
* format
* fix tests
* update submodules
* update PR for VTI
* consistency
* update submodules
* updated integratedTests submodule
* Updating schema
* Updating the integratedTests hash

---------

Co-authored-by: Julien Rene Thomas Besset <[email protected]>
Co-authored-by: acitrain <[email protected]>
Co-authored-by: Pavel Tomin <[email protected]>
Co-authored-by: Nicola Castelletto <[email protected]>
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 flag: requires rebaseline Requires rebaseline branch in integratedTests type: bug Something isn't working type: cleanup / refactor Non-functional change (NFC) type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants