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

[MISC, TEST] Restrict fpr to exclusive range (0.0, 1.0) and at bin_size_in_bits_test. #148

Merged
merged 2 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 6 additions & 1 deletion include/hibf/build/bin_size_in_bits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#pragma once

#include <cassert>
#include <cmath> // for log, ceil, exp
#include <cstddef> // for size_t

Expand All @@ -31,8 +32,12 @@ struct bin_size_parameters
/*!\brief Computes the bin size.
* \ingroup hibf_build
*/
inline size_t bin_size_in_bits(bin_size_parameters const & params)
inline constexpr size_t bin_size_in_bits(bin_size_parameters const & params)
{
assert(params.hash_count > 0);
assert(params.fpr > 0.0);
assert(params.fpr < 1.0);

double const numerator{-static_cast<double>(params.elements * params.hash_count)};
double const denominator{std::log(1 - std::exp(std::log(params.fpr) / params.hash_count))};
double const result{std::ceil(numerator / denominator)};
Expand Down
8 changes: 4 additions & 4 deletions include/hibf/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ struct config
* The internal Bloom Filters will be configured accordingly. Individual Bloom Filters might have a different
* but always lower false positive rate (FPR).
*
* Value must be in range [0,1].
* Value must be in range (0.0,1.0).
* Recommendation: default value (0.05)
*
* The FPR influences the memory consumption of the (H)IBF:
Expand Down Expand Up @@ -209,7 +209,7 @@ struct config
* The higher alpha, the less merged bins are chosen in the layout. This improves query times but leads to a bigger
* index.
*
* Value must be in range [0,max(double)].
* Value must be in range [0.0,max(double)].
* Recommendation: default value (1.2).
* disable_estimate_union
* Be sure to experiment with this option with your data before changing it.
Expand All @@ -225,7 +225,7 @@ struct config
* the larger the intervals. This potentially improves the layout, but increases the runtime of the layout
* algorithm.
*
* Value must be in range [0,1].
* Value must be in range [0.0,1.0].
* Recommendation: default value (0.5).
*/
double max_rearrangement_ratio{0.5};
Expand Down Expand Up @@ -264,7 +264,7 @@ struct config
*
* Constrains:
* * seqan::hibf::config::number_of_hash_functions must be in `[1,5]`.
* * seqan::hibf::config::maximum_false_positive_rate must be in `[0.0,1.0]`.
* * seqan::hibf::config::maximum_false_positive_rate must be in `(0.0,1.0)`.
* * seqan::hibf::config::threads must be greater than `0`.
* * seqan::hibf::config::sketch_bits must be in `[5,32]`.
* * seqan::hibf::config::tmax must be at most `18446744073709551552`.
Expand Down
4 changes: 2 additions & 2 deletions src/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ void config::validate_and_set_defaults()
if (number_of_hash_functions == 0u || number_of_hash_functions > 5u)
throw std::invalid_argument{"[HIBF CONFIG ERROR] config::number_of_hash_functions must be in [1,5]."};

if (maximum_false_positive_rate < 0.0 || maximum_false_positive_rate > 1.0)
throw std::invalid_argument{"[HIBF CONFIG ERROR] config::maximum_false_positive_rate must be in [0.0,1.0]."};
if (maximum_false_positive_rate <= 0.0 || maximum_false_positive_rate >= 1.0)
throw std::invalid_argument{"[HIBF CONFIG ERROR] config::maximum_false_positive_rate must be in (0.0,1.0)."};

if (threads == 0u)
throw std::invalid_argument{"[HIBF CONFIG ERROR] config::threads must be greater than 0."};
Expand Down
5 changes: 5 additions & 0 deletions test/unit/hibf/build/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# SPDX-FileCopyrightText: 2006-2023, Knut Reinert & Freie Universität Berlin
# SPDX-FileCopyrightText: 2016-2023, Knut Reinert & MPI für molekulare Genetik
# SPDX-License-Identifier: BSD-3-Clause

hibf_test (bin_size_in_bits_test.cpp)
30 changes: 30 additions & 0 deletions test/unit/hibf/build/bin_size_in_bits_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// SPDX-FileCopyrightText: 2006-2023, Knut Reinert & Freie Universität Berlin
// SPDX-FileCopyrightText: 2016-2023, Knut Reinert & MPI für molekulare Genetik
// SPDX-License-Identifier: BSD-3-Clause

#include <gtest/gtest.h> // for Message, TestPartResult, EXPECT_EQ, Test, ASSERT_EQ, TestInfo, TEST

#include <cstddef> // for size_t
#include <optional> // for optional
#include <vector> // for vector, allocator

#include <hibf/build/bin_size_in_bits.hpp> // for graph

TEST(bin_size_in_bits_test, general)
{
EXPECT_EQ((seqan::hibf::build::bin_size_in_bits({.fpr = 0.05, .hash_count = 2, .elements = 1000})), 7903u);
}

TEST(bin_size_in_bits_test, no_elements)
{
EXPECT_EQ((seqan::hibf::build::bin_size_in_bits({.fpr = 0.05, .hash_count = 1, .elements = 0})), 0u);
}

#ifndef NDEBUG
TEST(bin_size_in_bits_test, assert_trigger)
{
EXPECT_DEATH((seqan::hibf::build::bin_size_in_bits({.fpr = 0.05, .hash_count = 0, .elements = 10})), "");
EXPECT_DEATH((seqan::hibf::build::bin_size_in_bits({.fpr = 0.0, .hash_count = 2, .elements = 10})), "");
EXPECT_DEATH((seqan::hibf::build::bin_size_in_bits({.fpr = 1.0, .hash_count = 2, .elements = 10})), "");
}
#endif
10 changes: 5 additions & 5 deletions test/unit/hibf/config_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,17 @@ TEST(config_test, validate_and_set_defaults)
check_error_message(configuration, "[HIBF CONFIG ERROR] config::number_of_hash_functions must be in [1,5].");
}

// maximum_false_positive_rate must be in [0.0,1.0]
// maximum_false_positive_rate must be in (0.0,1.0)
{
seqan::hibf::config configuration{.input_fn = dummy_input_fn,
.number_of_user_bins = 1u,
.maximum_false_positive_rate = -0.1};
.maximum_false_positive_rate = 0.0};
check_error_message(configuration,
"[HIBF CONFIG ERROR] config::maximum_false_positive_rate must be in [0.0,1.0].");
"[HIBF CONFIG ERROR] config::maximum_false_positive_rate must be in (0.0,1.0).");

configuration.maximum_false_positive_rate = 1.1;
configuration.maximum_false_positive_rate = 1.0;
check_error_message(configuration,
"[HIBF CONFIG ERROR] config::maximum_false_positive_rate must be in [0.0,1.0].");
"[HIBF CONFIG ERROR] config::maximum_false_positive_rate must be in (0.0,1.0).");
}

// threads cannot be 0
Expand Down