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

CSTRS does not respect the thread safety attribute of the UDP #270

Open
bluescarni opened this issue Apr 28, 2019 · 0 comments
Open

CSTRS does not respect the thread safety attribute of the UDP #270

bluescarni opened this issue Apr 28, 2019 · 0 comments
Labels

Comments

@bluescarni
Copy link
Member

bluescarni commented Apr 28, 2019

The CSTRS meta problem ignores the thread safety level advertised by a UDP. Here is an example exhibiting the issue:

#include <future>

#include <pagmo/algorithms/cstrs_self_adaptive.hpp>
#include <pagmo/algorithms/de.hpp>
#include <pagmo/population.hpp>
#include <pagmo/threading.hpp>
#include <pagmo/types.hpp>

using namespace pagmo;

// Shitty udp writing to global variable
// during evolution.
struct my_shitty_udp
{
        vector_double fitness(const vector_double &) const
        {
                for (auto i = 0; i < 1000; ++i) {
                        s_vec.resize(100);
                        s_vec.resize(1);
                }
                return {0,0};
        }
        std::pair<vector_double, vector_double> get_bounds() const
        {
                return {{0}, {1}};
        }
        vector_double::size_type get_nic() const
        {
                return 1;
        }
        // Correctly advertises its shittiness.
        thread_safety get_thread_safety() const
        {
                return thread_safety::none;
        }
        static std::vector<double> s_vec;
};

std::vector<double> my_shitty_udp::s_vec;

struct my_uda
{
        population evolve(population pop) const
        {
                const auto &prob = pop.get_problem();
                if (prob.get_thread_safety() > thread_safety::none) {
                        auto runner = [pop]() {
                                return de{}.evolve(pop); 
                        };
                        auto h1 = std::async(std::launch::async, runner);
                        auto h2 = std::async(std::launch::async, runner);
                        auto h3 = std::async(std::launch::async, runner);
                        auto h4 = std::async(std::launch::async, runner);

                        h1.get();
                        h2.get();
                        h3.get();
                        return h4.get();
                } else {
                        return pop;
                }
        }
};

int main()
{
    my_shitty_udp udp;
    problem prob(udp);
    cstrs_self_adaptive uda(4, my_uda());
    algorithm algo(uda);
    population pop(prob, 10);
    algo.set_verbosity(1);
    pop = algo.evolve(pop);
}

Here we have a shitty UDP which, in its fitness(), writes to a global variable. The shitty UDP advertises its own shittiness by implementing the get_thread_safety() method with a thread_safety::none return value, meaning "please don't use me from multiple threads because I am a piece of crap".

Then we implement a UDA that, for non-shitty input problems, uses multiple threads to evolve different copies of the initial population.

When we use the shitty UDP and our new UDA with cstrs, there will be a crash with high likelihood. This is because the internal UDP constructed and used by cstrs does not implement the get_thread_safety() method, and thus pagmo assumes such internal udp offers at least the basic thread safety guarantee (which the shitty UDP does not). Consequently, when my_uda evolve() runs, it "sees" a problem with basic thread safety guarantees, and thus will run the multi-threaded codepath.

Fixing this in CSRTS requires both taking into account the thread safety of the original UDP, and reasoning about the thread safety implications of storing a naked pointer to the origina population object.

@bluescarni bluescarni added the bug label Apr 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant