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

Add option to build all examples with CMake, plus some minor fixes #2021

Merged
merged 12 commits into from
Nov 3, 2020

Conversation

ZedThree
Copy link
Member

  • Add BOUT_BUILD_EXAMPLES option to CMake to build all of the examples
  • Add README files for all examples (fixes READMEs for examples #390) (not necessarily very good or comprehensive READMEs, mind)
  • Fix for using the CMake build directory directly
  • Minor fixes for some of the examples:
    • conducting-wall-mode had multiple [laplace] sections in the input file
    • laplace-dae used auto instead of auto& for getting Options sections
    • remove a completely unused source file from advdiff2
  • Fix LaTeX typo in finite volume docs

One or two of the examples currently don't build; fixed in #1986

The CMake files for the examples are setup such that they can be built either altogether with the library, or individually as separate projects. I'm not completely happy with how I've done it here. When building them with the library, they get built as part of the default target (all), and the executables get put in the build directory in the usual mirrored-filesystem type way. This means building everything is trivial (nice for the maintainers), but it's less useful if you actually want to run the examples, as the path is now a bit more complicated.

Another way of doing this might be to copy the bout_add_integrated_test cmake function which also copies the data directory.
Or maybe even make sure all the options had the correct defaults, and modify the library so that data/BOUT.inp was entirely optional, making it possible to run without an input file?


It would be good to also compile all the examples on at least one of the Travis jobs, although that will need to wait till #1986 goes in

@ZedThree ZedThree added the build-system issues with make/configure/... label Apr 15, 2020
@ZedThree ZedThree added this to the BOUT-5.0 milestone Apr 15, 2020
@dschwoerer
Copy link
Contributor

Did compiling the example got much faster?
Last time I checked compiling (against the static library) took around 20 minutes and required 10 to 20 GB ...

I think that compiling the examples isn't particular useful, if they cannot be run. Even as a maintainer we should move in the direction to make sure that all the examples are run-able, splitting out the binary from the data dir is thus not a huge improvement, imho.

@ZedThree
Copy link
Member Author

Did compiling the example got much faster?
Last time I checked compiling (against the static library) took around 20 minutes and required 10 to 20 GB

I've not measured before/after, but it's comparable with building the library on the latest next. Using a shared library, the whole build directory is 708MB for me, and that's with debug symbols.

I think that compiling the examples isn't particular useful, if they cannot be run. Even as a maintainer we should move in the direction to make sure that all the examples are run-able, splitting out the binary from the data dir is thus not a huge improvement, imho

I'm not sure what you mean. Compiling the examples as part of the CI means we don't forget to update them when we make breaking changes, so it's definitely useful. I agree that running them to make sure they still work would be more useful, but there's still value in just compiling them.
Running them all should probably be part of the release process. I hope the test suite covers enough that they would break before the examples would.

We should probably have a refresh of the examples: remove outdated ones (advdiff?), remove duplicates, make sure they're all using the latest API, and so on.

@dschwoerer
Copy link
Contributor

I think that compiling the examples isn't particular useful, if they cannot be run. Even as a maintainer we should move in the direction to make sure that all the examples are run-able, splitting out the binary from the data dir is thus not a huge improvement, imho

I'm not sure what you mean. Compiling the examples as part of the CI means we don't forget to update them when we make breaking changes, so it's definitely useful. I agree that running them to make sure they still work would be more useful, but there's still value in just compiling them.

Sorry, probably wasn't clear. I am not arguing we shouldn't check that they compile, we should. I am just not sure exposing it this way to the user is useful. If I see this option and toggle it on, I will not be happy that they do not work. I would prefer to have a test similar to the configure system where we cd to each folder and check that we can compile against the current bout.

Running them all should probably be part of the release process. I hope the test suite covers enough that they would break before the examples would.

I started looking into that, but some examples require to select a specific dir, it might be useful to always have one pre-selected so it runs without options?

We should probably have a refresh of the examples: remove outdated ones (advdiff?), remove duplicates, make sure they're all using the latest API, and so on.

Sure, would be great. I think we have way to many examples.

* next: (161 commits)
  Enable staggered versions of SplitFluxDerivativeType
  make error look nicer
  rename to bout-add-mod-path and minor clean-up
  Revert name of legacy netCDF macro
  Fix typo making variable a tuple instead of string
  Tidy test-stopCheck runtest, don't limit to netcdf
  Fix for DataFile not returning scalars correctly with HDF5
  Use yup_index and ydown_index instead of mesh.ystart
  Cast Options to useful type for printing
  Use Region instead of unique_ptr<Region> for ZInterpolate.region
  Tighten set tolerance for LaplacePetsc3dAmg test
  Remove commented out code, temporary variable
  Use compile-time options instead of macros for default dump format
  Fix typo in help text
  Support multiple parallel slices in ShiftedMetricInterp
  Use custom region instead of "BoutMask skip_mask" in ZInterpolation
  [clang-format-command] fixes
  Update src/sys/boutexception.cxx
  Correct comment about parallel_slice_phases
  Use C++ style casts
  ...
Copies input files to build directory when building examples as part
of library
@ZedThree
Copy link
Member Author

I've now added bout_add_example that works like bout_add_integrated_test and copies the relevant input files into the build directory. It checks to see if the current directory is being built as part of the library or a standalone project. If it's standalone, it doesn't actually copy anything. This way all the examples can be built and run either individually or as part of building the main library.

This ends up being quite nice as it's now possible to keep separate builds of library + examples -- production and debug, say -- without cluttering up the examples/ directory, and makes cleaning up trivial.

Copy link
Contributor

@bendudson bendudson left a comment

Choose a reason for hiding this comment

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

Thanks @ZedThree !

@ZedThree ZedThree merged commit be8f44d into next Nov 3, 2020
@ZedThree ZedThree deleted the cmakeify-examples branch November 3, 2020 15:25
@ZedThree ZedThree mentioned this pull request Nov 11, 2020
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system issues with make/configure/...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants