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 or delete examples #2811

Merged
merged 43 commits into from
Oct 23, 2024
Merged

Fix or delete examples #2811

merged 43 commits into from
Oct 23, 2024

Conversation

bendudson
Copy link
Contributor

@bendudson bendudson commented Nov 14, 2023

Work in progress.

Aiming to clean up the examples, to ensure that they all work. I think the number of examples should also be reduced, either by deleting them or moving them to a separate repository.

We should also have little test in each example, that at least ensures that the example builds and runs a short step.

  • 2Dturbulence_multigrid
  • 6field-simple
  • advdiff
  • advdiff2
  • backtrace
  • blob2d
  • blob2d-laplacexz
  • blob2d-outerloop
  • boundary-conditions
  • boutcore
  • boutpp
  • conducting-wall-mode
  • conduction
  • conduction-snb
  • constraints
  • dalf3
  • eigen-box
  • elm-pb
  • elm-pb-outerloop
  • em-drift
  • fci-wave
  • fci-wave-logn
  • finite-volume
    • diffusion
    • fluid
    • test
  • gas-compress
  • gravity_reduced
  • gyro-gem
  • hasegawa-wakatani
  • hasegawa-wakatani-3d
  • IMEX
    • advection-diffusion
    • advection-reaction
    • diffusion-nl
    • drift-wave
    • drift-wave-constraint
  • invertable_operator
  • jorek-compare
  • lapd-drift
  • laplace-petsc3d
  • laplacexy
  • make-script
  • monitor
  • monitor-newapi
  • orszag-tang
  • performance
    • arithmetic
    • arithmetic_3d2d
    • bracket
    • communications
    • ddx
    • ddy
    • ddz
    • iterator
    • iterator-offsets
    • laplace
    • tuning_regionblocksize
  • preconditioning
    • wave
  • reconnect-2field
  • shear-alfven-wave
  • lstaggered_grid
  • subsampling
  • tokamak-2fluid
  • uedge-benchmark
  • wave-slab
  • zoidberg

bendudson and others added 8 commits November 14, 2023 11:23
The orszag-tang example allocates a field in the monitor.
Writing before the monitor is called therefore causes a
segfault.
Useful for allowing chaining of assignment and attributes,
tidies up physics model code writing output variables.
Uses the new output system, rather than storing pointers.
Should help avoid segfaults and uninitialised variables.
Build directory should contain a data/ subdirectory so that
running "blob2d" without arguments runs ok.

Copy the blob_velocity analysis script.
Quantities can be written to output with attributes like units
and conversion factors.
The sod-shock case runs, but I don't think the parameters are right.
The rayleigh-taylor case doesn't run. Not enough grid cells for
unclear reason.
Unused restart_format
@ZedThree
Copy link
Member

ZedThree commented Nov 15, 2023

Suggestions for deletion:

  • 2Dturbulence_multigrid
    • Demonstrates a solver, could done with an input file for a different model
    • Move to separate repo?
  • advdiff
    • I don't think this does what it says it does
  • advdiff2
    • Same as advdiff but split over multiple files -- not necessarily a useful example?
  • backtrace
    • Could just put the output in the docs?
  • blob2d-laplacexz
    • Possibly squash into blob2d?
  • boutcore
    • Think this has been replaced by boutpp? @dschwoerer ? edit: was incomplete move, resolved
  • em-drift
    • Similar model to others?
  • fci-wave-logn
    • Similar to fci-wave
  • laplace-petsc3d
    • Is this maybe supposed to be a test?
  • monitor
    • Old API -- should delete the API as well!
  • staggered_grid
    • Maybe a test?
  • subsampling
    • Maybe just documentation?
  • zoidberg
    • Should be in zoidberg repo?

Move to separate repos?

  • 6field-simple
  • dalf3
  • elm-pb[-outerloop]
  • gravity_reduced
  • gyro-gem
  • hasegawa-wakatani-3d
  • jorek-compare
  • lapd-drift
  • reconnect-2field
  • tokamak-2fluid
  • uedge-benchmark

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

examples/dalf3/dalf3.cxx Outdated Show resolved Hide resolved
examples/dalf3/dalf3.cxx Outdated Show resolved Hide resolved
examples/gas-compress/gas_compress.hxx Outdated Show resolved Hide resolved
examples/gas-compress/gas_compress.hxx Outdated Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
ZedThree and others added 6 commits January 19, 2024 16:42
* next: (111 commits)
  Apply clang-format changes
  Remove commented out code
  Improve comment
  prefer std::equal
  Remove propietary data
  Be more verbose by default
  Explicitly set -std=c++17 for --cflags
  Apply clang-format changes
  use std::optional<size_t>
  Add more OpenMP comments
  Add comments on OpenMP + mutex
  Fix escaping
  Remove python2 compatibility
  rename getDefaultRegion to getValidRegionWithDefault
  Apply clang-format changes
  Fix test-twistshift-staggered
  FieldFactory: Use string matching for options
  Apply clang-format changes
  Boolean expressions: Allow "True" and "False" again
  Apply clang-format changes
  ...
Remove `backtrace` example, move into docs instead
dschwoerer
dschwoerer previously approved these changes Mar 14, 2024
Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

I would vote to merge this now, and then open a new PR with more things later.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

include/bout/options.hxx Show resolved Hide resolved
include/bout/options.hxx Show resolved Hide resolved
include/bout/options.hxx Show resolved Hide resolved
include/bout/options.hxx Show resolved Hide resolved
include/bout/options.hxx Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
src/sys/options.cxx Show resolved Hide resolved
@dschwoerer
Copy link
Contributor

(sorry, I did a bad merge, and then a force push to fix it.)

@dschwoerer
Copy link
Contributor

I have a fix for fci-wave in my db-outer branch, but that will require some of my other open PRs to work ...
fci-wave has an example 'logn' - so I deleted fci-wave-logn ...

@ZedThree
Copy link
Member

Failing test is some problem with fedora packages

ZedThree and others added 16 commits September 12, 2024 09:15
* next: (146 commits)
  CMake: Replace include SourceRuns with SourceCompiles
  Fix typos in laplacian inversion documentation (credit @MigMadeira)
  Arkode solver: Change adap_method to an enum class
  Arkode solver: Add treatment enum, remove imex/implicit/explicit
  arkode solver: Clang format
  Deleted the last line
  Removed ASSERT1
  Update src/solver/impls/arkode/arkode.cxx
  Update src/solver/impls/arkode/arkode.cxx
  Update src/solver/impls/arkode/arkode.cxx
  Update src/solver/impls/arkode/arkode.cxx
  changed ARKODE config. to use string as the treatment
  include fmt/ranges.h for using fmt::join()
  mark fmt::format() const
  Add mxorder option back, now deprecated
  Apply clang-format changes
  Solver: Ensure nonlinear RHS called first
  Add CVODE version guard
  CVODE solver: Pass linear flag to rhs()
  Imex issue is resolved
  ...
Use reference to Options (not a copy). Required since changes in branch next-options-nocopy.
Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

I think we should go ahead a merge this. The situation is much better already

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@ZedThree ZedThree merged commit 608c7b5 into next Oct 23, 2024
28 checks passed
@ZedThree ZedThree deleted the fix-examples branch October 23, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants