gemfony / geneva Goto Github PK

benchmarks/geneva/GParallelisationOverhead does not run properly. This issue has been discovered when trying to look at the created root file. The file is incorrect. When running the both function calls
startReferenceMeasurement(gdif_ref, ab);
startParallelMeasurement(go_parallel, gdif_par, parallelExecutionTimes);
in the main thread without the thread-pool we see, that the startParallelMeasurement function throws an exception that is ignored by the main thread.

To Reproduce

1. compile the library.
2. Run the benchmark benchmarks/geneva/GParallelisationOverhead

In some use cases a user might want to use MPI in the evaluation function of an individual. This conflicts with the GMPIConsumerT because both library and user code would try to initialize and finalize MPI, which is not possible. We should create a type of optimizer that supports this functionality natively such that users do not need to program custom workarounds.

Suggest to add a small doxygen description to each member function, describing purpose and arguments
Originally posted by @rberlich in #5 (comment)

Can I just ask, how firm you are on keeping a ...deep and long doxygen description on everything?

While I agree, that 'c_size" does not transport any information in that case, maybe a nice varname and/or maybe with a short comment would already suffice.

I don't mind adding /*!< Enum value TVal1 */ or //!< a member function where needed (obv. always debatable). I just think there are a lot of methods/functions/ where you don't need detailed/very elaborated information, because function- and variable names (should) already convey a lot of the functionality.

Hello there,

I tried to compile the develop branch and there are some missing headers which make the build fail.

OS: ubuntu-22 LTS
g++ : 11.2
CXXEXTRAFLAGS : -O2 -Wall -march=native -flto

include/geneva/GPostProcessorT.hpp was missing the header
#include "boost/serialization/library_version_type.hpp"

include/courtier/GMPIConsumerT.hpp was missing an "optional" in the header list

include/common/GThreadGroup.hpp was missing a "memory" in the headers

/include/geneva/GMPISubClientParaboloidIndividualMultiD.hpp was missing an "optional"

when I added these it compiled ok. When I run prepareBuild.sh it says "Using C++ standard 17" but this seems to not be reflected in the compilation. Also in INSTALL it states that "Geneva currently requires support for C++14" so if I take that literally, std::optional wasn't added until c++17. Perhaps if c++17 is supposed to be used that should be updated in the INSTALL?

Also boost 1.74 is available via the synaptic package manager on my system, so maybe the statement "The required Boost version (1.70 or later) is currently not available as official OS packages in any of the above distributions" should be updated in the INSTALL.

Best,

Jamie

Current status:
Consumers are components that take raw work items from the queue of the broker, process them and finally put the processed items into a second queue of processed items. As of right now we have three types of consumers: A serial consumer processing work items on a single local thread, a multithreaded consumer processing items on multiple local threads which preferably run on different CPU-cores and a networked boost::asio consumer which acts as a server for boost::asio consumer clients. For more details refer to the official manual.

Problem domain:
At GSI geneva is used in a cluster environment. In this environment local execution with one or multiple cores of course is not enough. Therefore the boost::asio consumer has been used by GSI. It has been experienced that the boost::asio consumer might become a bottle neck. Therefore the proposal was made to create another consumer type, the MPI consumer, and compare its performance with the performance of the boost::asio consumer.
The mentioned performance bottle neck has been experienced to become increasingly severe with increasing number of clients connected to the boost::asio consumer as well as with decreasing evaluation time for single work items. This makes sense as both those factors directly increase the frequency of communication that is required between the consumer and its clients per unit of time.

Related issues:
The above explanation reduces the issue of a bottleneck in the consumer to the networking. However, that is only part of the issue. @rberlich has stated that he has experienced serialization taking up a major part of the total time of communication on side of the boost::asio consumer. The proposed solution therefore can possibly only solve one of the underlying two issues, which is the issue of networking overhead. As the serialization is not influenced by an MPI consumer it will therefore not improve the serialization time compared to a boost::asio consumer. Benchmarks will eventually show how much of an improvement the MPI consumer is. Another promising subsequent approach could be to improve the serialization of work items in order to require less computation power.

The desired implementation:
We want a solution for an MPI consumer and a corresponding client which is:

• functional
• hopefully more performant then the boost::asio consumer solution
• natively plugs into the existing broker of geneva
• is an abstraction such that the user does not need to know about implementation details, just like the boost::asio consumer
• scalable
• extensible

Status:

• Design Draft
• Implementation
• Tests and related bug fixes
  • Integration Tests
  • Tests in production environment
  • Benchmarks
• In-code documentation
• Review

As discussed via Mail, we want to adhere to some uniform coding styles via clang-format.
This issue shall be used as a discussion thread on how and where coding styles will be enforced.

Describe the solution you'd like
I'd like us to start with the google style as default and customize the rest to our wishes/needs.

Additional context
Should we past example functions here or rather create a PR on some example geneva code?
Maybe @rberlich or @agarcia-gmf know a good file where one might see a lot of the change a style would bring to the code base.

Also: Where and how should coding styles be enforced? ( PRs / git hooks/... )

Using MPI_Cancel for MPI_ISend is deprecated since MPI 4.0: https://www.mpi-forum.org/docs/mpi-4.0/mpi40-report.pdf#section.16.3
The GMPIConsumerT should be reworked to remove the MPI_Cancel calls for MPI_ISend requests.

When compiling with unit tests, i.e. when having set BUILDTESTCODE="1", the mpi consumer (and possibly other code as well) is broken. This refers to boost version 1.80.0.
Simple MPIConsumer examples then crash at the end of the run if compiled with test code.
I have noticed, that when building with test code, the whole library depends on the boost unit testing framework.
If possible, I think it would be better if only the unit tests are linked with the boost unit testing modules and the library itself as well as the tests do not depend on the boost unit testing module.
Another approach is to try different boost versions and eventually raise the minimum boost version.

The broker stores a vector of shared pointers to consumers.
The consumers store a shared pointer to the broker.
This cyclic shared pointer relationship creates a memory leak, because either object cannot be destructed when the shared pointer goes out of scope.
Proposed solution:
By using the GBROKER() macro instead of storing a member variable shared pointer to the broker in the consumers, we access temporary shared pointers that will be destroyed at the end of the respective statement, thereby not having any shared pointers to the brokers alive while the consumers are destructed. This breaks the cyclic dependency and resolves the problem

Goal is to create a benchmark that is capable of showing a performance comparison between networked consumers with arbitrary configuration.

The file extension ".cpp" must be specified in the CMAKE files when adding the sources to the example projects.
This apparently has been forgotten in issue #20 .

@rberlich
If one use for the consumers the new option asio(beast)_use_pinning=1 then the listener thread pool_size will be set to the maximum available cpus of the server node ( typically in our case it will be 64*2= 128 pinned threads).
The user-defined thread-pool size is then ignored.
My original code was only pinning each threads of the user-defined thread-pool to a single cpu if pool_size <= n_cpus.
Wouldn't it be the appropriate behaviour of the asio(beast)_use_pinning=1 option ?

Is your feature request related to a problem? Please describe.
In example GDirectEA one can build up their own executor for the old GAsioConsumerT, but not the new GWebsocketConsumerT as accessor methods are missing.This is not the first time I want to have accessors to some internal variables to modify behaviour in code. I think that access is often very restrictive, which it might need to be because of the order in which configuration parsing and simplified interfaces via go2 and defaults are done.

Describe alternatives you've considered
I think the best way in long term would be to rework geneva's configuration system to enable a (additional) change of configuration of all exported parameters via code (somewhere in GParseBuilder or the like). This in turn would reap many possible additional advantages, which would also allow to run geneva bins without any configuration file at all, while simultaniously remove the need for a lot of member accessors that are exported anyway.

additional context for alternatives
When trying to port geneva to different languages/systems it is not always easy to use/require config files, as they might not be the best fit. Otherwise, all exports will have to be written with extensive and error prone glue code like in GDirectEA.

Describe the solution you'd like
Right now a simple short term fix would be to add the missing methods for all needed parameters, but then again this might happen often.

ps: I swapped the template to give some focus. Maybe this should be 2 issues.

MPIConsumer crashes with segfault at end of program:

backtrace:

(gdb) backtrace
#0 0x00007fffef9a6acf in raise () from /lib64/libc.so.6
#1 0x00007fffef979ea5 in abort () from /lib64/libc.so.6
#2 0x00007fffef9e7cd7 in __libc_message () from /lib64/libc.so.6
#3 0x00007fffef9eefdc in malloc_printerr () from /lib64/libc.so.6
#4 0x00007fffef9efa44 in malloc_consolidate () from /lib64/libc.so.6
#5 0x00007fffef9f1090 in _int_free () from /lib64/libc.so.6
#6 0x00007fffdf6aca3f in kh_destroy_vfs_obj_inplace (h=)
at vfs/base/vfs_obj.c:79
#7 ucs_initializer_dtor0 () at vfs/base/vfs_obj.c:825
#8 0x00007ffff7dd6cee in _dl_fini () at dl-fini.c:141
#9 0x00007fffef9a926c in __run_exit_handlers () from /lib64/libc.so.6
#10 0x00007fffef9a93a0 in exit () from /lib64/libc.so.6
#11 0x00007fffef992d8c in __libc_start_main () from /lib64/libc.so.6
#12 0x00000000005219be in _start ()

Initializing MPISubclientOptimizer with subclientsize 1 should be allowed. Currently the constructor throws an exception when doing that.

A docker and docker-compose setup for quick test runs with the following features:

• Compile and run the Geneva Library in a docker container
• Build a ready-to-ship docker image that has geneva and all its dependencies installed
• Small local slurm cluster with docker-compose to test scheduling geneva on a slurm cluster without having access to a real cluster

Describe the bug
A clear and concise description of what the bug is.
compiling develop branch with gcc 8.3.0 leads to std::filesystem linking problem

To Reproduce
try compiling develop branch with gcc 8.3.0

Expected behavior
the project should compile

Desktop (please complete the following information):

• OS: [e.g. iOS] Linux Centos7
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Additional context
Linking error message may be to non full c++ 17 compliance of gcc 8.3.0, ( compiles with gcc 9.3 without errors )

In function `g_error_streamer::operator std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >() const':
GenevaStandardTests.cpp:(.text._ZNK16g_error_streamercvNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEv[_ZNK16g_error_streamercvNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEv]+0xa8): undefined reference to `std::filesystem::__cxx11::path::_M_split_cmpts()'
../../../src/common/libgemfony-common.so.1.10.0: undefined reference to `std::filesystem::__cxx11::path::remove_filename()'
../../../src/common/libgemfony-common.so.1.10.0: undefined reference to `std::filesystem::__cxx11::path::_M_find_extension() const'
../../../src/geneva/libgemfony-geneva.so.1.10.0: undefined reference to `std::filesystem::last_write_time(std::filesystem::__cxx11::path const&)'
../../../src/geneva/libgemfony-geneva.so.1.10.0: undefined reference to `std::filesystem::__cxx11::path::has_root_directory() const'
../../../src/geneva/libgemfony-geneva.so.1.10.0: undefined reference to `std::filesystem::__cxx11::path::has_filename() const'
../../../src/geneva-individuals/libgemfony-geneva-individuals.so.1.10.0: undefined reference to `std::filesystem::remove(std::filesystem::__cxx11::path const&)'
../../../src/geneva-individuals/libgemfony-geneva-individuals.so.1.10.0: undefined reference to `std::filesystem::status(std::filesystem::__cxx11::path const&)'
../../../src/geneva/libgemfony-geneva.so.1.10.0: undefined reference to `std::filesystem::create_directory(std::filesystem::__cxx11::path const&)'
../../../src/common/libgemfony-common.so.1.10.0: undefined reference to `std::filesystem::__cxx11::path::compare(std::filesystem::__cxx11::path const&) const'
../../../src/geneva/libgemfony-geneva.so.1.10.0: undefined reference to `std::filesystem::rename(std::filesystem::__cxx11::path const&, std::filesystem::__cxx11::path const&)'

Hi Denis, I believe you are working on an MPI consumer. Any success and comparison with the ASIO/Beast-variants?

GMPISubClientConsumer has been broken by merging issue #26, because the number of stop requests required in GMPIConsumerT is not calculated correctly due to not setting the communicator to the intercommunication properly.