Skip to content

gui: change HistogramView bucket sizes #9564

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

Merged
maliberty merged 4 commits into from
Mar 2, 2026
Merged

gui: change HistogramView bucket sizes #9564

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
78fc108
gui: change HistogramView bucket sizes
LucasYuki Feb 27, 2026
d89c00d
gui: code refactoring
LucasYuki Feb 27, 2026
c5b0e0d
gpl: fix HistogramView::populateBins 0 slack corner case
LucasYuki Feb 27, 2026
b1b2d51
Merge remote-tracking branch 'private/master' into gui_histogram
LucasYuki Mar 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
84 changes: 29 additions & 55 deletions src/gui/src/chartsWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,14 +632,19 @@ void HistogramView::populateBins()
return;
}

const float min_slack = std::min(0.0f, histogram_->getMinValue());
const float max_slack = std::max(0.0f, histogram_->getMaxValue());
if (min_slack == max_slack) {
precision_count_ = 1;
histogram_->generateBins(1, -0.1, 0.2);
return;
}

// determine interval
const float bin_interval = computeBucketInterval();
const float bin_min
= std::floor(std::min(0.0f, histogram_->getMinValue()) / bin_interval)
* bin_interval;
const float bin_max
= std::ceil(std::max(0.0f, histogram_->getMaxValue()) / bin_interval)
* bin_interval;
const float bin_interval = computeBucketInterval(min_slack, max_slack);
const float bin_min = std::floor(min_slack / bin_interval) * bin_interval;
const float bin_max = std::ceil(max_slack / bin_interval) * bin_interval;

const int bins = (bin_max - bin_min) / bin_interval;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Feb 27, 2026

Choose a reason for hiding this comment

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

critical

If min_slack and max_slack are equal, bin_min can equal bin_max. This results in bins being 0 due to integer truncation. Passing 0 for the number of bins to utl::Histogram will cause a crash later when getMaxBinCount() is called on an empty bins vector. You should ensure there's at least one bin. Using std::round is also safer for floating-point-to-integer conversion.

Suggested change
const int bins = (bin_max - bin_min) / bin_interval;
const int bins = std::max(1, static_cast<int>(std::round((bin_max - bin_min) / bin_interval)));

histogram_->generateBins(bins, bin_min, bin_interval);
}
Expand Down Expand Up @@ -717,66 +722,35 @@ void HistogramView::setData(const EndPointSlackMap& data, sta::ClockSet* clocks)
setVisualConfig();
}

float HistogramView::computeBucketInterval()
float HistogramView::computeBucketInterval(float min_slack, float max_slack)
{
float min_slack = histogram_->getMinValue();
float max_slack = histogram_->getMaxValue();

if (min_slack < 0 && max_slack < 0) {
max_slack = 0;
} else if (min_slack > 0 && max_slack > 0) {
min_slack = 0;
}

const float exact_interval
= (max_slack - min_slack) / kDefaultNumberOfBuckets;

const float snap_interval = computeSnapBucketInterval(exact_interval);

// We compute a new number of buckets based on the snap interval.
const int new_number_of_buckets = (max_slack - min_slack) / snap_interval;

if (new_number_of_buckets < kMinimumNumberOfBuckets) {
const float minimum_interval
= (max_slack - min_slack) / kMinimumNumberOfBuckets;

float decimal_snap_interval
= computeSnapBucketDecimalInterval(minimum_interval);

return decimal_snap_interval;
}
return snap_interval;
}

float HistogramView::computeSnapBucketDecimalInterval(float minimum_interval)
{
float integer_part = minimum_interval;
int power_count = 0;

while (static_cast<int>(integer_part) == 0) {
integer_part *= 10;
++power_count;
}

precision_count_ = power_count;

return std::ceil(integer_part) / std::pow(10, power_count);
}

float HistogramView::computeSnapBucketInterval(float exact_interval)
{
if (exact_interval < 10) {
return std::ceil(exact_interval);
}

float snap_interval = 0;
const int digits = computeNumberOfDigits(exact_interval);

while (snap_interval < exact_interval) {
snap_interval += 5 * std::pow(10, digits - 2);
int exp = std::floor(std::log10(exact_interval));
precision_count_ = std::max(-exp, 0);

double mag = std::pow(10.0, exp);
double residual = exact_interval / mag;

double nice_coeff;
if (residual < 1.5) {
nice_coeff = 1.0;
} else if (residual < 3.0) {
nice_coeff = 2.0;
} else if (residual < 7.0) {
nice_coeff = 5.0;
} else {
nice_coeff = 10.0;
}

return snap_interval;
return nice_coeff * mag;
}
Comment on lines 735 to 754
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Feb 27, 2026

Choose a reason for hiding this comment

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

critical

If exact_interval is zero or negative (which can happen if max_slack equals min_slack), calling std::log10 will result in an error (-inf or NaN). This propagates through the calculations and can lead to a crash. Please add a guard to handle this edge case.

{
  if (exact_interval <= 0.0f) {
    return 1.0f;
  }
  int exp = std::floor(std::log10(exact_interval));
  precision_count_ = std::max(-exp, 0);

  double mag = std::pow(10.0, exp);
  double residual = exact_interval / mag;

  double nice_coeff;
  if (residual < 1.5) {
    nice_coeff = 1.0;
  } else if (residual < 3.0) {
    nice_coeff = 2.0;
  } else if (residual < 7.0) {
    nice_coeff = 5.0;
  } else {
    nice_coeff = 10.0;
  }

  return nice_coeff * mag;
}


int HistogramView::computeNumberOfDigits(float value)
Expand Down
6 changes: 2 additions & 4 deletions src/gui/src/chartsWidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ class HistogramView : public QChartView
void contextMenuEvent(QContextMenuEvent* event) override;

private:
float computeBucketInterval();
float computeBucketInterval(float min_slack, float max_slack);
float computeSnapBucketInterval(float exact_interval);
float computeSnapBucketDecimalInterval(float minimum_interval);
int computeNumberOfDigits(float value);

void populateBins();
Expand Down Expand Up @@ -118,8 +117,7 @@ class HistogramView : public QChartView
Buckets buckets_;
std::unique_ptr<utl::Histogram<float>> histogram_;

static constexpr int kDefaultNumberOfBuckets = 15;
static constexpr int kMinimumNumberOfBuckets = 8;
static constexpr int kDefaultNumberOfBuckets = 10;
};

class ChartsWidget : public QDockWidget
Expand Down