Skip to content

Commit

Permalink
Try fixing nightmare bug
Browse files Browse the repository at this point in the history
  • Loading branch information
@reiniscirpons
reiniscirpons committed Oct 23, 2024
1 parent 3ca89b9 commit bf32dd0
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 141 deletions.
114 changes: 66 additions & 48 deletions include/libsemigroups/sims.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,13 @@
// #include <filesystem> // for path, create_directories, temp_directory_path
#include <functional> // for function
#include <iterator> // for forward_iterator_tag
#include <mutex> // for mutex
#include <string> // for operator+, basic_string
#include <utility> // for move
#include <vector> // for vector
#include <libsemigroups/detail/report.hpp>
#include <libsemigroups/knuth-bendix.hpp>
#include <mutex> // for mutex
#include <string> // for operator+, basic_string
#include <thread>
#include <utility> // for move
#include <vector> // for vector

#include <fstream>

Expand Down Expand Up @@ -183,7 +186,21 @@ namespace libsemigroups {
//! \no_libsemigroups_except
SimsStats();

// TODO(0) add an init function
//! \brief Reinitialize an existing SimsStats object.
//!
//! This function puts a SimsStats object back into the same state as if
//! it had been newly default constructed.
//!
//! \parameters (None)
//!
//! \returns A reference to \c this.
//!
//! \exception
//! \no_libsemigroups_except
SimsStats& init() {
stats_zero();
return *this;
}

//! Copy constructor.
//!
Expand All @@ -196,7 +213,7 @@ namespace libsemigroups {
//! \exceptions
//! \no_libsemigroups_except
SimsStats(SimsStats const& that) : SimsStats() {
init_from(that);
init(that);
}

//! Copy assignment operator.
Expand All @@ -210,7 +227,7 @@ namespace libsemigroups {
//! \exceptions
//! \no_libsemigroups_except
SimsStats& operator=(SimsStats const& that) {
return init_from(that);
return init(that);
}

//! Move constructor.
Expand All @@ -224,7 +241,7 @@ namespace libsemigroups {
//! \exceptions
//! \no_libsemigroups_except
SimsStats(SimsStats&& that) : SimsStats() {
init_from(that);
init(that);
}

//! Move assignment operator.
Expand All @@ -238,9 +255,12 @@ namespace libsemigroups {
//! \exceptions
//! \no_libsemigroups_except
SimsStats& operator=(SimsStats&& that) {
return init_from(that);
return init(that);
}

//! Initialize from SimsStats.
SimsStats& init(SimsStats const& that);

//! \brief Set all statistics to zero.
//!
//! Set all statistics to zero.
Expand All @@ -266,10 +286,6 @@ namespace libsemigroups {
total_pending_last = total_pending_now.load();
return *this;
}

private:
// TODO(0) should this be public and called just "init"?
SimsStats& init_from(SimsStats const& that);
};

//! \ingroup congruences_group
Expand Down Expand Up @@ -456,9 +472,9 @@ namespace libsemigroups {
//!
//! \throws LibsemigroupsException if the argument \p val is 0.
//!
//! \warning If \p val exceeds `std::thread::hardware_concurrency()`, then
//! this is likely to have a negative impact on the performance of the
//! algorithms implemented by Sims1 or Sims2.
//! \note The value of \p val is capped at
//! `std::thread::hardware_concurrency()`. Trying to set a higher value is
//! equivalent to setting `std::thread::hardware_concurrency()`.
Subclass& number_of_threads(size_t val);

//! \brief Get the number of threads.
Expand Down Expand Up @@ -670,6 +686,10 @@ namespace libsemigroups {
//! \noexcept
//!
//! \sa \ref pruners
//!
//! \warning When running the Sims low-index backtrack with multiple
//! threads, each added pruner must be guaranteed thread safe. Failing to do
//! so could cause bad things to happen.
template <typename Func>
Subclass& add_pruner(Func&& func) {
_pruners.emplace_back(func);
Expand Down Expand Up @@ -1000,9 +1020,6 @@ namespace libsemigroups {
}

private:
template <typename OtherSubclass>
SimsSettings& init_from(SimsSettings<OtherSubclass> const& that);

template <typename Iterator>
Subclass& include_exclude(Iterator first,
Iterator last,
Expand Down Expand Up @@ -1409,7 +1426,6 @@ namespace libsemigroups {

public:
//! Default constructor
// TODO(0) (doc)
Sims1() = default;

// TODO(0) (doc)
Expand Down Expand Up @@ -3224,22 +3240,29 @@ namespace libsemigroups {
//! \sa \ref SimsSettings::add_pruner
class SimsRefinerIdeals {
private:
using node_type = uint32_t;
KnuthBendix<> _knuth_bendix;
using node_type = uint32_t;
using KnuthBendix_ = KnuthBendix<detail::RewriteFromLeft>;
std::vector<KnuthBendix_> _knuth_bendices;

public:
//! Default constructor.
explicit SimsRefinerIdeals() : _knuth_bendix(congruence_kind::twosided) {}
explicit SimsRefinerIdeals()
: _knuth_bendices(std::thread::hardware_concurrency() + 1,
KnuthBendix_(congruence_kind::twosided)) {
init();
}

//! \brief Reinitialize an existing SimsRefinerIdeals object.
//!
//! This function puts an object back into the same state as if it had
//! been newly default constructed.
//!
//! \returns A reference to \c *this.
template <typename Word>
SimsRefinerIdeals& init() {
_knuth_bendix.init(congruence_kind::twosided);
_knuth_bendices[0].init(congruence_kind::twosided).run();
std::fill(_knuth_bendices.begin() + 1,
_knuth_bendices.end(),
_knuth_bendices[0]);
return *this;
}

Expand All @@ -3254,7 +3277,10 @@ namespace libsemigroups {
//! terminate on certain inputs.
template <typename Word>
explicit SimsRefinerIdeals(Presentation<Word> const& p)
: _knuth_bendix(congruence_kind::twosided, p) {}
: _knuth_bendices(std::thread::hardware_concurrency() + 1,
KnuthBendix_(congruence_kind::twosided)) {
init(p);
}

//! \brief Reinitialize an existing SimsRefinerIdeals object from a
//! presentation.
Expand All @@ -3277,11 +3303,12 @@ namespace libsemigroups {
//! terminate on certain inputs.
//!
//! \sa presentation(Presentation<std::string> const&)
// TODO(0): template on typename Word so that we can handle string and
// word_type in a single function
template <typename Word>
SimsRefinerIdeals& init(Presentation<Word> const& p) {
_knuth_bendix.init(congruence_kind::twosided, p);
_knuth_bendices[0].init(congruence_kind::twosided, p).run();
std::fill(_knuth_bendices.begin() + 1,
_knuth_bendices.end(),
_knuth_bendices[0]);
return *this;
}

Expand All @@ -3300,7 +3327,7 @@ namespace libsemigroups {
//! \noexcept
[[nodiscard]] Presentation<std::string> const&
presentation() const noexcept {
return _knuth_bendix.presentation();
return _knuth_bendices[0].presentation();
}

//! \brief Check if a word graph can be extended to one defining a Rees
Expand All @@ -3315,15 +3342,22 @@ namespace libsemigroups {
//! presentation that was used to construct the SimsRefinerIdeals object. If
//! this is not the case then th pruner may not terminate on certain inputs.
bool operator()(Sims1::word_graph_type const& wg) {
// TODO(2) Make knuth bendix thread safe to use here without the bodge
using sims::right_generating_pairs_no_checks;
_knuth_bendix.run();

node_type sink = UNDEFINED;

LIBSEMIGROUPS_ASSERT(detail::this_threads_id()
< std::thread::hardware_concurrency() + 1);
// TODO(1) change this to be const& when we have const_contains for knuth
// bendix
auto& kb = _knuth_bendices[detail::this_threads_id()];
for (auto const& p : right_generating_pairs_no_checks(wg)) {
auto const& u = p.first;
auto const& v = p.second;
if (!_knuth_bendix.contains(u, v)) {
// TODO(1) change this to be const_contains for knuth
// bendix when we have it
if (!kb.contains(u, v)) {
auto beta
= word_graph::follow_path_no_checks(wg, 0, u.cbegin(), u.cend());
if (sink == UNDEFINED) {
Expand Down Expand Up @@ -3363,22 +3397,6 @@ namespace libsemigroups {
//! \no_libsemigroups_except
[[nodiscard]] std::string to_human_readable_repr(SimsStats const& x);

// TODO(0): Remove
// //! \ingroup congruence_group
// //!
// //! \brief Return a human readable representation of a SimsSettings object.
// //!
// //! Return a human readable representation of a SimsSettings object.
// //!
// //! \param x the SimsSettings object.
// //!
// //! \exceptions
// //! \no_libsemigroups_except
// // TODO(1) to tpp
// template <typename Subclass>
// [[nodiscard]] std::string
// target size 0 and to_human_readable_repr(SimsSettings<Subclass> const& x);

//! \ingroup congruence_group
//!
//! \brief Return a human readable representation of a Sims1 object.
Expand Down
35 changes: 13 additions & 22 deletions src/sims.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ namespace libsemigroups {
return *this;
}

SimsStats& SimsStats::init_from(SimsStats const& that) {
SimsStats& SimsStats::init(SimsStats const& that) {
count_last = that.count_last.load();
count_now = that.count_now.load();
max_pending = that.max_pending.load();
Expand Down Expand Up @@ -117,27 +117,8 @@ namespace libsemigroups {
// access.
_longs_begin = _presentation.rules.cend();

// TODO(0) does not seem to be a way of clearing _stats
// _stats.init();

// TODO(0) move to exclude function
// auto pruner = [this](auto const& wg) {
// auto first = _exclude.cbegin();
// auto last = _exclude.cend();
// node_type root = 0;
//
// for (auto it = first; it != last; it += 2) {
// auto l = word_graph::follow_path_no_checks(wg, root, *it);
// if (l != UNDEFINED) {
// auto r = word_graph::follow_path_no_checks(wg, root, *(it + 1));
// if (l == r) {
// return false;
// }
// }
// }
// return true;
// };
// add_pruner(pruner);
_stats.init();

return static_cast<Subclass&>(*this);
}

Expand Down Expand Up @@ -169,6 +150,11 @@ namespace libsemigroups {
LIBSEMIGROUPS_EXCEPTION(
"the argument (number of threads) must be non-zero");
}

if ((std::thread::hardware_concurrency() > 0)
&& (val > std::thread::hardware_concurrency())) {
val = std::thread::hardware_concurrency();
}
_num_threads = val;
return static_cast<Subclass&>(*this);
}
Expand Down Expand Up @@ -939,6 +925,8 @@ namespace libsemigroups {
that._pending.cend());
}

// TODO(0) expand to longer comment about thread sanitizer warnings and how
// we can gleefully ignore them
template <typename Sims1or2>
bool SimsBase<Sims1or2>::thread_iterator::try_steal(thread_iterator& that) {
std::lock_guard<std::mutex> lock(iterator_base::_mtx);
Expand Down Expand Up @@ -1040,6 +1028,9 @@ namespace libsemigroups {
_done = true;
throw;
}
// TODO(2) Implement a ThreadIdGuard so that thread ids are automatically
// reset after they exit scope.
detail::reset_thread_ids();
}

////////////////////////////////////////////////////////////////////////
Expand Down
Loading

0 comments on commit bf32dd0

Please sign in to comment.