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

refactor: fix conversion warnings in v_array and removed deprecated usages #4310

Merged
merged 5 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions vowpalwabbit/core/include/vw/core/v_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
const_iterator cbegin() const noexcept { return _begin; }
const_iterator cend() const noexcept { return _end; }

v_array() noexcept : _begin(nullptr), _end(nullptr), _end_array(nullptr), _erase_count(0) {}
v_array() noexcept : _begin(nullptr), _end(nullptr), _end_array(nullptr) {}
~v_array() { delete_v_array(); }

v_array(v_array<T>&& other) noexcept
Expand Down Expand Up @@ -115,9 +115,21 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::

bool empty() const { return _begin == _end; }

T& operator[](size_t i) const { return _begin[i]; }
size_t size() const { return _end - _begin; }
size_t capacity() const { return _end_array - _begin; }
T& operator[](size_t i) const
{
assert(i < size());
return _begin[i];
}
size_t size() const
{
assert(_end >= _begin);
return static_cast<size_t>(_end - _begin);
}
size_t capacity() const
{
assert(_end_array >= _begin);
return static_cast<size_t>(_end_array - _begin);
}

/**
* \brief Change the size of the container.
Expand Down Expand Up @@ -193,7 +205,7 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::

const size_t idx = it - begin();
destruct_item(it);
memmove(&_begin[idx], &_begin[idx + 1], (size() - (idx + 1)) * sizeof(T));
std::memmove(&_begin[idx], &_begin[idx + 1], (size() - (idx + 1)) * sizeof(T));
--_end;
return begin() + idx;
}
Expand All @@ -215,7 +227,7 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
const size_t first_index = first - begin();
const size_t num_to_erase = last - first;
for (auto current = first; current != last; ++current) { destruct_item(current); }
memmove(
std::memmove(
&_begin[first_index], &_begin[first_index + num_to_erase], (size() - (first_index + num_to_erase)) * sizeof(T));
_end -= num_to_erase;
return begin() + first_index;
Expand Down Expand Up @@ -264,8 +276,9 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
{
assert(it >= begin());
assert(it <= end());
const size_t idx = it - begin();
const auto num_elements = std::distance(first, last);
const auto idx = static_cast<size_t>(it - begin());
assert(first >= last);
const auto num_elements = static_cast<size_t>(std::distance(first, last));
make_space_at(idx, num_elements);
std::copy(first, last, begin() + idx);
}
Expand Down Expand Up @@ -308,7 +321,7 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
static constexpr size_t ERASE_POINT = ~((1u << 10u) - 1u);

template <typename S, typename std::enable_if<std::is_trivially_destructible<S>::value, bool>::type = true>
static void destruct_item(S*)
static void destruct_item(S* /* unused */)
{
// If S is trivially destructive nothing needs to be done.
}
Expand All @@ -324,7 +337,7 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
if (_begin != nullptr)
{
for (iterator item = _begin; item != _end; ++item) { destruct_item(item); }
free(_begin);
std::free(_begin);
}
_begin = nullptr;
_end = nullptr;
Expand All @@ -346,7 +359,8 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::

_end = _begin + std::min(old_len, length);
_end_array = _begin + length;
memset(static_cast<void*>(_end), 0, (_end_array - _end) * sizeof(T));
assert(_end_array >= _end);
std::memset(static_cast<void*>(_end), 0, static_cast<size_t>(_end_array - _end) * sizeof(T));
}

// This will move all elements after idx by width positions and reallocate the underlying buffer if needed.
Expand All @@ -355,8 +369,8 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
if (width > 0)
{
if (size() + width > capacity()) { reserve_nocheck(2 * capacity() + width); }
memmove(&_begin[idx + width], &_begin[idx], (size() - idx) * sizeof(T));
memset(&_begin[idx], 0, width * sizeof(T));
std::memmove(&_begin[idx + width], &_begin[idx], (size() - idx) * sizeof(T));
std::memset(&_begin[idx], 0, width * sizeof(T));
_end += width;
}
}
Expand All @@ -379,11 +393,12 @@ class v_array<T, typename std::enable_if<std::is_trivially_copyable<T>::value>::
T* _begin;
T* _end;
T* _end_array;
size_t _erase_count;
size_t _erase_count{};
};

} // namespace VW

// This is deprecated. Cannot mark templates as deprecated though so a message must suffice.
// VW_DEPRECATED: This is deprecated. Cannot mark templates as deprecated though so a message must suffice.
// TODO: remove this alias.
template <typename T>
using v_array = VW::v_array<T>;
6 changes: 3 additions & 3 deletions vowpalwabbit/core/src/reductions/cb/cb_explore_adf_bag.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace
class cb_explore_adf_bag
{
public:
using PredictionT = v_array<VW::action_score>;
using PredictionT = VW::v_array<VW::action_score>;

cb_explore_adf_bag(
float epsilon, size_t bag_size, bool greedify, bool first_only, std::shared_ptr<VW::rand_state> random_state);
Expand All @@ -51,7 +51,7 @@ class cb_explore_adf_bag
bool _first_only;
std::shared_ptr<VW::rand_state> _random_state;

v_array<VW::action_score> _action_probs;
VW::v_array<VW::action_score> _action_probs;
std::vector<float> _scores;
std::vector<float> _top_actions;
uint32_t get_bag_learner_update_count(uint32_t learner_index);
Expand All @@ -78,7 +78,7 @@ uint32_t cb_explore_adf_bag::get_bag_learner_update_count(uint32_t learner_index
void cb_explore_adf_bag::predict(VW::LEARNER::multi_learner& base, VW::multi_ex& examples)
{
// Randomize over predictions from a base set of predictors
v_array<VW::action_score>& preds = examples[0]->pred.a_s;
VW::v_array<VW::action_score>& preds = examples[0]->pred.a_s;
uint32_t num_actions = static_cast<uint32_t>(examples.size());
if (num_actions == 0)
{
Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/core/src/reductions/cb/cb_explore_adf_cover.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class cb_explore_adf_cover
VW::version_struct _model_file_version;
VW::io::logger _logger;

v_array<VW::action_score> _action_probs;
VW::v_array<VW::action_score> _action_probs;
std::vector<float> _scores;
VW::cs_label _cs_labels;
VW::cs_label _cs_labels_2;
Expand Down Expand Up @@ -125,7 +125,7 @@ void cb_explore_adf_cover::predict_or_learn_impl(VW::LEARNER::multi_learner& bas
GEN_CS::gen_cs_example_ips(examples, _cs_labels, _logger);
VW::LEARNER::multiline_learn_or_predict<false>(base, examples, examples[0]->ft_offset);
}
v_array<VW::action_score>& preds = examples[0]->pred.a_s;
VW::v_array<VW::action_score>& preds = examples[0]->pred.a_s;
const uint32_t num_actions = static_cast<uint32_t>(preds.size());

float additive_probability = 1.f / static_cast<float>(_cover_size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void cb_explore_adf_first::predict_or_learn_impl(multi_learner& base, VW::multi_
if (is_learn) { multiline_learn_or_predict<true>(base, examples, examples[0]->ft_offset); }
else { multiline_learn_or_predict<false>(base, examples, examples[0]->ft_offset); }

v_array<VW::action_score>& preds = examples[0]->pred.a_s;
VW::v_array<VW::action_score>& preds = examples[0]->pred.a_s;
uint32_t num_actions = static_cast<uint32_t>(preds.size());

if (_tau)
Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/core/src/reductions/cb/cb_explore_adf_regcb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ void cb_explore_adf_regcb::get_cost_ranges(float delta, multi_learner& base, VW:
void cb_explore_adf_regcb::predict_impl(multi_learner& base, VW::multi_ex& examples)
{
multiline_learn_or_predict<false>(base, examples, examples[0]->ft_offset);
v_array<VW::action_score>& preds = examples[0]->pred.a_s;
VW::v_array<VW::action_score>& preds = examples[0]->pred.a_s;
uint32_t num_actions = static_cast<uint32_t>(preds.size());

const float max_range = _max_cb_cost - _min_cb_cost;
Expand Down Expand Up @@ -216,7 +216,7 @@ void cb_explore_adf_regcb::predict_impl(multi_learner& base, VW::multi_ex& examp

void cb_explore_adf_regcb::learn_impl(multi_learner& base, VW::multi_ex& examples)
{
v_array<VW::action_score> preds = std::move(examples[0]->pred.a_s);
VW::v_array<VW::action_score> preds = std::move(examples[0]->pred.a_s);
for (size_t i = 0; i < examples.size() - 1; ++i)
{
CB::label& ld = examples[i]->l.cb;
Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/core/src/reductions/cb/cb_explore_adf_rnd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class cb_explore_adf_rnd
void zero_bonuses(VW::multi_ex&);
void accumulate_bonuses(VW::multi_ex&);
void finish_bonuses();
void compute_ci(v_array<VW::action_score>&, float);
void compute_ci(VW::v_array<VW::action_score>&, float);

template <bool>
void save_labels(VW::multi_ex&);
Expand Down Expand Up @@ -109,7 +109,7 @@ void cb_explore_adf_rnd::finish_bonuses()
for (auto& b : _bonuses) { b = std::sqrt(b / _numrnd); }
}

void cb_explore_adf_rnd::compute_ci(v_array<VW::action_score>& preds, float max_bonus)
void cb_explore_adf_rnd::compute_ci(VW::v_array<VW::action_score>& preds, float max_bonus)
{
constexpr float eulergamma = 0.57721566490153286f;
for (auto& p : preds) { p.score -= eulergamma * (_bonuses[p.action] - max_bonus); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void cb_explore_adf_softmax::predict_or_learn_impl(VW::LEARNER::multi_learner& b
{
VW::LEARNER::multiline_learn_or_predict<is_learn>(base, examples, examples[0]->ft_offset);

v_array<VW::action_score>& preds = examples[0]->pred.a_s;
VW::v_array<VW::action_score>& preds = examples[0]->pred.a_s;
exploration::generate_softmax(
-_lambda, begin_scores(preds), end_scores(preds), begin_scores(preds), end_scores(preds));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void cb_explore_adf_squarecb::predict(multi_learner& base, VW::multi_ex& example
{
multiline_learn_or_predict<false>(base, examples, examples[0]->ft_offset);

v_array<VW::action_score>& preds = examples[0]->pred.a_s;
VW::v_array<VW::action_score>& preds = examples[0]->pred.a_s;
uint32_t num_actions = static_cast<uint32_t>(preds.size());

// The actual parameter $\gamma$ used in the SquareCB.
Expand Down Expand Up @@ -269,7 +269,7 @@ void cb_explore_adf_squarecb::predict(multi_learner& base, VW::multi_ex& example

void cb_explore_adf_squarecb::learn(multi_learner& base, VW::multi_ex& examples)
{
v_array<VW::action_score> preds = std::move(examples[0]->pred.a_s);
VW::v_array<VW::action_score> preds = std::move(examples[0]->pred.a_s);
for (size_t i = 0; i < examples.size() - 1; ++i)
{
CB::label& ld = examples[i]->l.cb;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class cb_explore_adf_synthcover

VW::version_struct _model_file_version;

v_array<VW::action_score> _action_probs;
VW::v_array<VW::action_score> _action_probs;
float _min_cost;
float _max_cost;
template <bool is_learn>
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/core/src/reductions/slates.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ void update_stats_slates(const VW::workspace& /* all */, shared_data& sd, const
float loss = 0.;
bool is_labelled = ec_seq[VW::details::SHARED_EX_INDEX]->l.slates.labeled;
float cost = is_labelled ? ec_seq[VW::details::SHARED_EX_INDEX]->l.slates.cost : 0.f;
v_array<VW::action_score> label_probs;
VW::v_array<VW::action_score> label_probs;

for (auto* ec : ec_seq)
{
Expand Down
6 changes: 3 additions & 3 deletions vowpalwabbit/core/src/reductions/topk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace
class topk
{
public:
using container_t = std::multimap<float, v_array<char>>;
using container_t = std::multimap<float, VW::v_array<char>>;
using const_iterator_t = container_t::const_iterator;
topk(uint32_t k_num);

Expand All @@ -32,7 +32,7 @@ class topk
void clear_container();

private:
void update_priority_queue(float pred, v_array<char>& tag);
void update_priority_queue(float pred, const VW::v_array<char>& tag);

const uint32_t _k_num;
container_t _pr_queue;
Expand All @@ -58,7 +58,7 @@ void topk::learn(VW::LEARNER::single_learner& base, VW::multi_ex& ec_seq)
}
}

void topk::update_priority_queue(float pred, v_array<char>& tag)
void topk::update_priority_queue(float pred, const VW::v_array<char>& tag)
{
if (_pr_queue.size() < _k_num) { _pr_queue.insert({pred, tag}); }
else if (_pr_queue.begin()->first < pred)
Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/core/tests/cache_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST(cache_tests, write_and_read_large_example)

TEST(cache_tests, write_and_read_tag)
{
v_array<char> tag;
VW::v_array<char> tag;
tag.push_back('m');
tag.push_back('y');
tag.push_back(' ');
Expand All @@ -120,7 +120,7 @@ TEST(cache_tests, write_and_read_tag)
io_buf io_reader;
io_reader.add_file(VW::io::create_buffer_view(backing_vector->data(), backing_vector->size()));

v_array<char> read_tag;
VW::v_array<char> read_tag;
VW::details::read_cached_tag(io_reader, read_tag);

EXPECT_THAT(tag, Pointwise(Eq(), read_tag));
Expand Down