Skip to content

Simplify proximity_counter by inheritance from bin_counter #125

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.

Sign up for GitHub

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

Draft
Minsoo-Kang-space wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Simplify proximity_counter by inheritance from bin_counter #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f979740
Enhanced Logging Model Content from ramtares_main
Minsoo-Kang-space Sep 23, 2026
4289021
Code cleanup; name parameter for apply_tolerance(), relocate header, …
Oct 1, 2026
812b571
Remove redundant initializer
Minsoo-Kang-space Oct 2, 2026
c35635d
Change empty constructor to default constructor
Minsoo-Kang-space Oct 2, 2026
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
4 changes: 2 additions & 2 deletions models/utilities/bin_counter/docs/bin_counter.odt
View file
Open in desktop
Git LFS file not shown
Binary file modified models/utilities/bin_counter/docs/bin_counter.pdf
View file
Open in desktop
Binary file not shown.
12 changes: 9 additions & 3 deletions models/utilities/bin_counter/include/bin_counter.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@ CML_BinCounterElement
Purpose: Contents of each element of the working vector
*****************************************************************************/
struct CML_BinCounterElement {
double value; /* (--)
Target value to be counted, used by proximity counter */
double bin_floor; /* (--)
Bin lower bound.*/
double bin_ceil; /* (--)
Bin upper bound. */
unsigned int count; /* (--)
Counter. Incremented when falling in this specific bin. */
unsigned int count; /* (1)
Counter. Incremented when falling in this specific bin.
For Proximity element, incremented when value is the closest one among
CML_BinCounterElement vector to the variable. */
};

/*****************************************************************************
Expand Down Expand Up @@ -73,13 +77,15 @@ class CML_BinCounter {
void set_data( double limit_a,
double limit_b,
unsigned int num_bins,
bool closed_ends = true);
bool closed_ends = true);
void apply_tolerance(double tol);
template <size_t n_edges>
void set_data( const double (&edges)[n_edges],
bool closed_ends = true)
{
std::vector<double> edges_v( edges, edges+n_edges);
set_data( edges_v, closed_ends);
}
size_t get_nbin() const {return nbin;}
};
#endif
34 changes: 5 additions & 29 deletions models/utilities/bin_counter/include/proximity_counter.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ PURPOSE:
times some variable of type double has a value that falls closest to each
of a set of target values.)

ASSUMPTIONS:
(Target values are monotonically increasing)

LIBRARY DEPENDENCY:
(../src/proximity_counter.cc)

PROGRAMMERS:
(
((Matthew Elmer, Gary Turner) (OSR) (Jun 2023)
(Refactored content from GNC_PAR; Overhauled grok_bin_counter))
((Hansen Lian) (OSR) (July 2026) (Inherits from BinCounter))
)
*******************************************************************************/
#ifndef CML_PROXIMITY_COUNTER_HH
Expand All @@ -23,50 +21,28 @@ PROGRAMMERS:
#include <string>
#include <vector>

/*****************************************************************************
CML_ProximityCounterElement
Purpose: Contents of each element of the working vector
*****************************************************************************/
struct CML_ProximityCounterElement {
double value; /* (--)
Target value to be counted */
unsigned int count; /* (--)
Counter. Incremented when value is the closest one among
CML_ProximityCounterElement vector to the variable. */
};

#include "cml/models/utilities/bin_counter/include/bin_counter.hh"

/*****************************************************************************
CML_ProximityCounter
Purpose:
Receives a value via the insert() command and bins it into one of
a set of bins, each covering a finite domain.
*****************************************************************************/
class CML_ProximityCounter {
protected:
std::vector<CML_ProximityCounterElement> targets; /* (--)
The target values. */
bool targets_ready; /* (--)
Check on presence of data in the targets array.*/
size_t ntarget; /* (--)
Number of targets.*/
const CML_ProximityCounterElement* target_data; /* (--)
Array of targets data. For logging compatibility only.*/

class CML_ProximityCounter : public CML_BinCounter {
public:
std::string name; /* (--)
Name of group (for debugging purposes)*/
CML_ProximityCounter();
explicit CML_ProximityCounter(const std::vector<double> & targets_);
CML_ProximityCounter(const CML_ProximityCounter&) = delete;
CML_ProximityCounter& operator=(const CML_ProximityCounter&) = delete;
void insert(double value);
using CML_BinCounter::insert;

@ninotarantino ninotarantino Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessary. This line can be removed.

void set_data( const std::vector<double> & targets_);
template <size_t n_targets>
void set_data( const double (&new_targets)[n_targets])
{
std::vector<double> targets_v( new_targets, new_targets+n_targets);
set_data( targets_v);
}
size_t get_ntarget() const {return nbin;}
};
#endif
3 changes: 2 additions & 1 deletion models/utilities/bin_counter/include/target_counter.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Purpose: Contents of each element of the working vector
struct CML_TargetCounterElement {
int value; /* (--)
Target integer value to be counted */
unsigned int count; /* (--)
unsigned int count; /* (1)
Counter. Incremented when variable = value. */
};

Expand Down Expand Up @@ -69,5 +69,6 @@ class CML_TargetCounter {
std::vector<int> targets_v( targets_, targets_+n_targets);
set_data( targets_v);
}
size_t get_ntarget() const {return ntarget;}
};
#endif
40 changes: 36 additions & 4 deletions models/utilities/bin_counter/src/bin_counter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,44 @@ CML_BinCounter::insert(double value)
// Consider only values below upper edge and only if model passed sanity
// check.
if (bins_ready && value <= bins[nbin-1].bin_ceil) {
for (int ii = static_cast<int>(nbin) - 1; ii >= 0; ii--) {
const auto bin_index = static_cast<size_t>(ii);
if (value >= bins[bin_index].bin_floor) {
bins[bin_index].count++;
//ii-- is a post decrement. tests ii > 0, then decrements.
//so index ii runs from nbin - 1 to 0.
for (auto bin = bins.rbegin(), end = bins.rend(); bin != end; ++bin) {
if (value >= bin->bin_floor) {
bin->count++;
return;
}
}
Comment thread
Minsoo-Kang-space marked this conversation as resolved.
}
}


/*****************************************************************************
Method: apply_tolerance
Purpose:
Modifies the specified bin edges, shifting them down slightly.
The lower bin edge behaves as a closed end, so a value is in the bin if it
is >= lower_edge. To allow for numerical rounding / truncation, it may be
desirable in some circumstances to extend that boundary.
For example, if the boundaries are set at {0, 1, 2} then a value of 0.9999999
would be binned into the lower bin. It may be desirable to include values
arbitrarily close to 1.0 into the upper bin and doing so requires lowering
the bin edge to accommodate that desired allowance.
*****************************************************************************/
void
CML_BinCounter::apply_tolerance(double tol)

@ninotarantino ninotarantino Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function needs to be tested.

{
if (!bins_ready) {
CMLMessage::error( __FILE__,__LINE__,
"Cannot apply bin tolerance, bins not ready\n");
return;
}

if (bins[0].bin_floor != std::numeric_limits<double>::lowest()) {
bins[0].bin_floor -= tol;
}
for (size_t ii = 0; ii < nbin; ii++) {
bins[ii].bin_ceil =
bins[ii+1].bin_floor = bins[ii].bin_ceil - tol;
}
Comment on lines +246 to +252

@ninotarantino ninotarantino Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an out-of-bounds read when ii == nbin - 1 on the last iteration.

All this loop is intending to do is to decrement the floor and ceiling of bins which aren't the first or last bin by tol. There's no reason to assign ceilings to floors. This could be rewritten to something like

// Double-check, this probably only works if nbin >= 2
for (auto bin = std::next(bins.begin()), end = std::prev(bins.end()); bin != end; ++bin) {
  bin->bin_floor -= tol;
  bin->bin_ceil -= tol;
}
bins[0].bin_ceil -= tol;
bins[nbin - 1].bin_floor -= tol;

if (bins[0].bin_floor != std::numeric_limits<double>::lowest()) {
  bins[0].bin_floor -= tol;
  bins[nbin - 1].bin_ceil -= tol;
}

Regardless, there's definitely future work here to better determine if the dataset has overflow bins or not. Comparing against the lowest double should be a static analysis warning and it's not obvious to someone reading this function why you're comparing against std::numeric_limits<double>::lowest() at first glance. Realistically the boundaries should just be infinity because subtracting a finite number will never change them and you can directly tell that they're overflow bins with std::isinf(). But like I said, that's a refactor for a separate PR.

}
104 changes: 28 additions & 76 deletions models/utilities/bin_counter/src/proximity_counter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,28 @@ LIBRARY DEPENDENCIES:
(
((Matthew Elmer, Gary Turner) (OSR) (Jun 2023)
(Refactored content from GNC_PAR; Overhauled grok_target_counter))
((Hansen Lian) (OSR) (July 2026) (Inherits from BinCounter))
)
*******************************************************************************/

#include "../include/proximity_counter.hh"
#include "cml/models/utilities/cml_message/include/cml_message.hh"
#include <cstddef>
#include <vector>
#include <limits>
#include <algorithm>

/*****************************************************************************
Constructors
*****************************************************************************/
CML_ProximityCounter::CML_ProximityCounter()
:
targets_ready(false),
ntarget(),
target_data(nullptr)
{}
CML_ProximityCounter::CML_ProximityCounter() = default;
/****************************************************************************/
CML_ProximityCounter::CML_ProximityCounter(
const std::vector<double> & targets_)
:
CML_ProximityCounter()
{
set_data(targets_);
CML_ProximityCounter::set_data(targets_);
}

/*****************************************************************************
Expand All @@ -46,18 +44,16 @@ set_data
Sets the edge data for constructing the targets. Deletes all previous
target structure and counts for those targets.
Options:
- Pass in a STL-vector of n edge values, there will be (n-1) targets from
these n values
- Pass in a C-style array of n edge values, there will be (n-1) targets from
these n values
- Pass in an upper limit and a lower limit and the number of targets;
targets will be equally spaced ebtween the two limits.
- Pass in a STL-vector of n target values, there will be (n) targets from
these n values with bins spaced at the midpoint of consecutive targets.
- Pass in a C-style array of n target values, there will be (n) targets from
these n values with bins spaced at the midpoint of consecutive target
*****************************************************************************/
void
CML_ProximityCounter::set_data(const std::vector<double> & targets_)
{
targets.clear();
targets_ready = false;
bins.clear();
bins_ready = false;

const size_t n_targets_ = targets_.size();
// Sanity check for number of targets:
Expand All @@ -68,69 +64,25 @@ CML_ProximityCounter::set_data(const std::vector<double> & targets_)
return;
}

// Sanity check for monotonicity of targets_:
for (size_t ii = 1; ii < n_targets_; ii++) {
if (targets_[ii] <= targets_[ii-1]) {
CMLMessage::error( __FILE__,__LINE__,
"Bin targets_ (target-group name : ",name,") are malformed,\n"
"they are not monotonically increasing.\n"
" edge(",ii-1,") = ",targets_[ii-1],"\n"
" edge(",ii, ") = ",targets_[ii], "\n"
"Group (",name,") cannot be used.\n");
return;
}
}

ntarget = n_targets_;
targets.resize(ntarget);
targets_ready = true;
target_data = targets.data();
for (size_t ii = 0; ii < ntarget; ii++) {
targets[ii].value = targets_[ii];
targets[ii].count = 0;
}
}
std::vector<double> sorted_targets = targets_;

/*****************************************************************************
insert
Purpose:Increments the count in the target in which the specified value falls.
Note:
Check each target on the half-open interval [L, R) except for the last, which
is a fully closed interval [L, R].
Processing upward from lower limit requires special treatment of last
target (all but last upper bound are open).
Processing downward from upper limit does not (all lower bounds are
closed) so we do that.
*****************************************************************************/
void
CML_ProximityCounter::insert(double value)
{
// Consider only values below upper edge and only if model passed sanity
// check.
if (!targets_ready) {return;}
// Sort the local copy
std::sort(sorted_targets.begin(), sorted_targets.end());

if (value < targets[0].value) {
targets[0].count++;
}
else if (value >= targets[ntarget-1].value) {
targets[ntarget-1].count++;
}
else {
size_t ix_search = 0;
while (ix_search < ntarget-2 &&
value > targets[ix_search+1].value) {
ix_search++;
}
// ix_search is between 0 and ntarget-2
// value lies between targets[ix_search] and targets[ix_search+1]
// Find the difference between value and the two bounding target values.
const double delta_low = value - targets[ix_search].value;
const double delta_high = targets[ix_search+1].value - value;
// increment the count for the closer target.
if (delta_high > delta_low) {
targets[ix_search].count++;
} else {
targets[ix_search+1].count++;
}
nbin = n_targets_;
bins.resize(nbin);
bins_ready = true;
bin_data = bins.data();
bins[0].bin_floor = std::numeric_limits<double>::lowest();
for (size_t ii = 0; ii < nbin-1; ii++) {
bins[ii].value = sorted_targets[ii];
bins[ii].count = 0;
const double bin_boundary = 0.5 * (sorted_targets[ii] + sorted_targets[ii+1]);
bins[ii].bin_ceil = bin_boundary;
bins[ii + 1].bin_floor = bin_boundary;
}
bins[nbin - 1].value = sorted_targets[nbin-1];
bins[nbin - 1].count = 0;
bins[nbin - 1].bin_ceil = std::numeric_limits<double>::max();
}
7 changes: 0 additions & 7 deletions models/utilities/bin_counter/verif/SIM_verif/S_define
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,6 @@ class AntaresUnitTestSimObject: public Trick::SimObject {

prox_counter.set_data( empty_dbl);

std::cout << "\n"
"***************************************************************************\n"
"Error - cannot create proximity counter with non monotonic data set.\n"
"***************************************************************************\n" << std::endl;

prox_counter.set_data( non_monot_edges);


// BIN COUNTER WARNINGS
std::cout << "\n"
Expand Down
2 changes: 1 addition & 1 deletion models/utilities/bin_counter/verif/SIM_verif/verif_data/RUN_01_bin/log_test_data.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
sys.exec.out.time {s},test.bin_counter_vec.bin_data[0].bin_floor {--},test.bin_counter_vec.bin_data[0].count {--},test.bin_counter_vec.bin_data[1].bin_floor {--},test.bin_counter_vec.bin_data[1].count {--},test.bin_counter_vec.bin_data[2].bin_floor {--},test.bin_counter_vec.bin_data[2].count {--},test.bin_counter_arr.bin_data[0].bin_floor {--},test.bin_counter_arr.bin_data[0].count {--},test.bin_counter_arr.bin_data[1].bin_floor {--},test.bin_counter_arr.bin_data[1].count {--},test.bin_counter_arr.bin_data[2].bin_floor {--},test.bin_counter_arr.bin_data[2].count {--},test.bin_counter_arr_open.bin_data[0].bin_floor {--},test.bin_counter_arr_open.bin_data[0].count {--},test.bin_counter_arr_open.bin_data[1].bin_floor {--},test.bin_counter_arr_open.bin_data[1].count {--},test.bin_counter_arr_open.bin_data[2].bin_floor {--},test.bin_counter_arr_open.bin_data[2].count {--},test.bin_counter_arr_open.bin_data[3].bin_floor {--},test.bin_counter_arr_open.bin_data[3].count {--},test.bin_counter_arr_open.bin_data[4].bin_floor {--},test.bin_counter_arr_open.bin_data[4].count {--},test.bin_counter_interval.bin_data[0].bin_floor {--},test.bin_counter_interval.bin_data[0].count {--},test.bin_counter_interval.bin_data[1].bin_floor {--},test.bin_counter_interval.bin_data[1].count {--},test.bin_counter_interval.bin_data[2].bin_floor {--},test.bin_counter_interval.bin_data[2].count {--},test.bin_counter_interval_open.bin_data[0].bin_floor {--},test.bin_counter_interval_open.bin_data[0].count {--},test.bin_counter_interval_open.bin_data[1].bin_floor {--},test.bin_counter_interval_open.bin_data[1].count {--},test.bin_counter_interval_open.bin_data[2].bin_floor {--},test.bin_counter_interval_open.bin_data[2].count {--},test.bin_counter_interval_open.bin_data[3].bin_floor {--},test.bin_counter_interval_open.bin_data[3].count {--},test.bin_counter_interval_open.bin_data[4].bin_floor {--},test.bin_counter_interval_open.bin_data[4].count {--}
sys.exec.out.time {s},test.bin_counter_vec.bin_data[0].bin_floor {--},test.bin_counter_vec.bin_data[0].count {1},test.bin_counter_vec.bin_data[1].bin_floor {--},test.bin_counter_vec.bin_data[1].count {1},test.bin_counter_vec.bin_data[2].bin_floor {--},test.bin_counter_vec.bin_data[2].count {1},test.bin_counter_arr.bin_data[0].bin_floor {--},test.bin_counter_arr.bin_data[0].count {1},test.bin_counter_arr.bin_data[1].bin_floor {--},test.bin_counter_arr.bin_data[1].count {1},test.bin_counter_arr.bin_data[2].bin_floor {--},test.bin_counter_arr.bin_data[2].count {1},test.bin_counter_arr_open.bin_data[0].bin_floor {--},test.bin_counter_arr_open.bin_data[0].count {1},test.bin_counter_arr_open.bin_data[1].bin_floor {--},test.bin_counter_arr_open.bin_data[1].count {1},test.bin_counter_arr_open.bin_data[2].bin_floor {--},test.bin_counter_arr_open.bin_data[2].count {1},test.bin_counter_arr_open.bin_data[3].bin_floor {--},test.bin_counter_arr_open.bin_data[3].count {1},test.bin_counter_arr_open.bin_data[4].bin_floor {--},test.bin_counter_arr_open.bin_data[4].count {1},test.bin_counter_interval.bin_data[0].bin_floor {--},test.bin_counter_interval.bin_data[0].count {1},test.bin_counter_interval.bin_data[1].bin_floor {--},test.bin_counter_interval.bin_data[1].count {1},test.bin_counter_interval.bin_data[2].bin_floor {--},test.bin_counter_interval.bin_data[2].count {1},test.bin_counter_interval_open.bin_data[0].bin_floor {--},test.bin_counter_interval_open.bin_data[0].count {1},test.bin_counter_interval_open.bin_data[1].bin_floor {--},test.bin_counter_interval_open.bin_data[1].count {1},test.bin_counter_interval_open.bin_data[2].bin_floor {--},test.bin_counter_interval_open.bin_data[2].count {1},test.bin_counter_interval_open.bin_data[3].bin_floor {--},test.bin_counter_interval_open.bin_data[3].count {1},test.bin_counter_interval_open.bin_data[4].bin_floor {--},test.bin_counter_interval_open.bin_data[4].count {1}ys.exec.out.time {s},test.bin_counter_vec.bin_data[0].bin_floor {--},test.bin_counter_vec.bin_data[0].count {--},test.bin_counter_vec.bin_data[1].bin_floor {--},test.bin_counter_vec.bin_data[1].count {--},test.bin_counter_vec.bin_data[2].bin_floor {--},test.bin_counter_vec.bin_data[2].count {--},test.bin_counter_arr.bin_data[0].bin_floor {--},test.bin_counter_arr.bin_data[0].count {--},test.bin_counter_arr.bin_data[1].bin_floor {--},test.bin_counter_arr.bin_data[1].count {--},test.bin_counter_arr.bin_data[2].bin_floor {--},test.bin_counter_arr.bin_data[2].count {--},test.bin_counter_arr_open.bin_data[0].bin_floor {--},test.bin_counter_arr_open.bin_data[0].count {--},test.bin_counter_arr_open.bin_data[1].bin_floor {--},test.bin_counter_arr_open.bin_data[1].count {--},test.bin_counter_arr_open.bin_data[2].bin_floor {--},test.bin_counter_arr_open.bin_data[2].count {--},test.bin_counter_arr_open.bin_data[3].bin_floor {--},test.bin_counter_arr_open.bin_data[3].count {--},test.bin_counter_arr_open.bin_data[4].bin_floor {--},test.bin_counter_arr_open.bin_data[4].count {--},test.bin_counter_interval.bin_data[0].bin_floor {--},test.bin_counter_interval.bin_data[0].count {--},test.bin_counter_interval.bin_data[1].bin_floor {--},test.bin_counter_interval.bin_data[1].count {--},test.bin_counter_interval.bin_data[2].bin_floor {--},test.bin_counter_interval.bin_data[2].count {--},test.bin_counter_interval_open.bin_data[0].bin_floor {--},test.bin_counter_interval_open.bin_data[0].count {--},test.bin_counter_interval_open.bin_data[1].bin_floor {--},test.bin_counter_interval_open.bin_data[1].count {--},test.bin_counter_interval_open.bin_data[2].bin_floor {--},test.bin_counter_interval_open.bin_data[2].count {--},test.bin_counter_interval_open.bin_data[3].bin_floor {--},test.bin_counter_interval_open.bin_data[3].count {--},test.bin_counter_interval_open.bin_data[4].bin_floor {--},test.bin_counter_interval_open.bin_data[4].count {--}
0, -1,1, 0.5,0, 2.1,0, -1,1, 0.5,0, 2.1,0,-1.797693134862316e+308,0, -1,1, 0.5,0, 2.1,0, 3.3,0, -1,1, 0.4333333333333333,0, 1.866666666666667,0,-1.797693134862316e+308,0, -1,1, 0.4333333333333333,0, 1.866666666666667,0, 3.3,0
1, -1,1, 0.5,0, 2.1,0, -1,1, 0.5,0, 2.1,0,-1.797693134862316e+308,1, -1,1, 0.5,0, 2.1,0, 3.3,0, -1,1, 0.4333333333333333,0, 1.866666666666667,0,-1.797693134862316e+308,1, -1,1, 0.4333333333333333,0, 1.866666666666667,0, 3.3,0
2, -1,2, 0.5,0, 2.1,0, -1,2, 0.5,0, 2.1,0,-1.797693134862316e+308,1, -1,2, 0.5,0, 2.1,0, 3.3,0, -1,2, 0.4333333333333333,0, 1.866666666666667,0,-1.797693134862316e+308,1, -1,2, 0.4333333333333333,0, 1.866666666666667,0, 3.3,0
Expand Down
Loading
Loading