How to bind nested vectors?

[nuttyartist]

So I have this function that works well.

std::vector< std::pair< std::vector<std::string>, std::vector<std::string> > > testAllItems(const std::vector< std::vector<std::string> >& list1, const std::vector< std::vector<std::string> >& list2) {
    std::vector< std::pair<std::vector<std::string>, std::vector<std::string>> > allPairs;
    for (auto item1 : list1) {
        for (auto item2 : list2) {
            if(doSomething(item1, item2, 1)) {
                std::pair<std::vector<std::string>, std::vector<std::string>> pair;
                pair.first = item1;
                pair.second = item2;
                allPairs.push_back(pair);
            }
        }
    }
    return allPairs;
}

PYBIND11_MODULE(module_name, module_handle) {
  module_handle.doc() = "I'm a docstring hehe";
  module_handle.def("check_all", &testAllItems);
}

But returning allPairs takes a long time. I think it's the overhead of pybind11 copying data into Python. I've tried to return a reference via py::return_value_policy::reference but I'm getting an error on the Python side. Does someone know how I can return this data structure correctly std::vector< std::pair< std::vector<std::string>, std::vector<std::string> > >?

[james-d-mitchell]

You know that writing:

for (auto item1 : list1) {

copies the entries in list1, right? So, you are copying every entry in list1 at least once, every entry in list2 at least list1.size() times, the assignments

pair.first = item1;
pair.second = item2;

copy item1 and item2 again (probably), and allPairs.push_back(pair); copies it a third time. So you're potentially doing 3 * list1.size() * list2.size() copies for nothing. Maybe try:

std::vector< std::pair< std::vector<std::string>, std::vector<std::string> > > testAllItems(const std::vector< std::vector<std::string> >& list1, const std::vector< std::vector<std::string> >& list2) {
    std::vector< std::pair<std::vector<std::string>, std::vector<std::string>> > allPairs;
    for (auto const& item1 : list1) {
        for (auto const& item2 : list2) {
            if(doSomething(item1, item2, 1)) {
               allPairs.emplace(item1, item2);
            }
        }
    }
    return allPairs;
}

[nuttyartist]

I've changed this line to

vector<vector<string>> get_phonemesList() const {

And modified this:

vector<pair<RhymeableTerm, RhymeableTerm>> testAllRhymeableTerms(const vector<RhymeableTerm> & allRhymeableTerms1, const vector<RhymeableTerm> & allRhymeableTerms2) {
    vector<pair<RhymeableTerm, RhymeableTerm>> allPairs;; // Computation take 23sec in Python when returning this
    for (auto const& rhymeableTerm1 : allRhymeableTerms1) {
        for (auto const& rhymeableTerm2 : allRhymeableTerms2) {
            for(auto phonemes1 : rhymeableTerm1.get_phonemesList()) {
                for(auto phonemes2 : rhymeableTerm2.get_phonemesList()) {
                    if(isPerfectRhyme(phonemes1, phonemes2, 1)) {
                        pair<RhymeableTerm,RhymeableTerm> pairNew(rhymeableTerm1, rhymeableTerm2);
                        allPairs.emplace(pairNew);
                        continue;
                    }
                }
            }
        }
    }
    return allPairs;
}

But this error remain:

main.cpp:135:34: error: no matching member function for call to 'emplace'
                        allPairs.emplace(pairNew);

[james-d-mitchell]

should be allPairs.emplace_back(rhymeableTerm1, rhymeableTerm2); I think and pairNew is not required.

[nuttyartist]

emplace results in error, but emplace_back works. But it still takes a long time to execute (still I believe it's the overhead of the data), is there a way to return a reference without Python copying all the data?

[james-d-mitchell]

get_phonemesList returns by value too which seems unnecessary. Basically, I don't think this is an issue with pybind11 more like an issue with C++, you should maybe try profiling your code to see where it is spending its time.

[nuttyartist]

You're right! I fixed the code by passing pointers rather than copies of the objects.
But I profiled, and even just iterating through the loop costs 2.5 seconds (out of 5s with the code inside the loops).
Do you see any more unnecessary copying?

vector<pair<RhymeableTerm*, RhymeableTerm*>> testAllRhymeableTerms(const vector<RhymeableTerm*> & allRhymeableTerms1, const vector<RhymeableTerm*> & allRhymeableTerms2) {
    vector<pair<RhymeableTerm*, RhymeableTerm*>> allPairs;
    for (auto rhymeableTerm1 : allRhymeableTerms1) {
        for (auto rhymeableTerm2 : allRhymeableTerms2) {
            for(auto phonemes1 : rhymeableTerm1->get_phonemesList()) {
                for(auto phonemes2 : rhymeableTerm2->get_phonemesList()) {
                    if(isPerfectRhyme(phonemes1, phonemes2, 1)) {
                        allPairs.push_back(make_pair(rhymeableTerm1, rhymeableTerm2));
                        continue;
                    }
                }
            }
        }
    }
    return allPairs;
}