Skip to content

Clamp probabilities passed to binomial distribution to $[0.0, 1.0]$. #86

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
niklas-uhl merged 4 commits into from
Dec 4, 2025
Merged

Clamp probabilities passed to binomial distribution to $[0.0, 1.0]$. #86

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
f090cbc
Clamp probabilities passed to binomial distribution to $[0.0, 1.0]$.
niklas-uhl Dec 4, 2025
4fe2737
Fix compilation.
niklas-uhl Dec 4, 2025
28d0a73
Use spaces instead of tabs.
niklas-uhl Dec 4, 2025
78dbf4a
Fix erroneous upper bound in one of the clamp calls.
niklas-uhl Dec 4, 2025
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: 3 additions & 1 deletion kagen/generators/geometric/geometric_2d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "libmorton/morton2D.h"
#include <tuple>
#include <vector>
#include <algorithm>

namespace kagen {
class Geometric2D : public virtual Generator, private EdgeListOnlyGenerator {
Expand Down Expand Up @@ -206,7 +207,8 @@ class Geometric2D : public virtual Generator, private EdgeListOnlyGenerator {
for (SInt i = 0; i < cells_per_chunk_; ++i) {
seed = config_.seed + chunk_id * cells_per_chunk_ + i + total_chunks_ * cells_per_chunk_;
SInt h = sampling::Spooky::hash(seed);
SInt cell_vertices = rng_.GenerateBinomial(h, n, cell_area / total_area);
// due to potential floating point inaccuracies clamp probability
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[nitpick] For type safety and consistency, consider using explicit type casting for the clamp bounds. Since cell_area and total_area are of type LPFloat, use std::clamp(cell_area / total_area, LPFloat{0.0}, LPFloat{1.0}) to match the pattern used in hyperbolic.cpp with Double{0.0} and Double{1.0}.

Suggested change
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, LPFloat{0.0}, LPFloat{1.0}));

Copilot uses AI. Check for mistakes.
LPFloat cell_start_x = std::get<1>(chunk) + (i / cells_per_dim_) * cell_size_;
LPFloat cell_start_y = std::get<2>(chunk) + (i % cells_per_dim_) * cell_size_;
if (cell_vertices != 0) {
Expand Down
3 changes: 2 additions & 1 deletion kagen/generators/geometric/geometric_3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "libmorton/morton3D.h"
#include <tuple>
#include <vector>
#include <algorithm>

namespace kagen {
class Geometric3D : public virtual Generator, private EdgeListOnlyGenerator {
Expand Down Expand Up @@ -247,7 +248,7 @@ class Geometric3D : public virtual Generator, private EdgeListOnlyGenerator {
for (SInt i = 0; i < cells_per_chunk_; ++i) {
seed = config_.seed + chunk_id * cells_per_chunk_ + i + total_chunks_ * cells_per_chunk_;
SInt h = sampling::Spooky::hash(seed);
SInt cell_vertices = rng_.GenerateBinomial(h, n, cell_area / total_area);
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[nitpick] For type safety and consistency, consider using explicit type casting for the clamp bounds. Since cell_area and total_area are of type LPFloat, use std::clamp(cell_area / total_area, LPFloat{0.0}, LPFloat{1.0}) to match the pattern used in hyperbolic.cpp with Double{0.0} and Double{1.0}.

Suggested change
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, LPFloat{0.0}, LPFloat{1.0}));

Copilot uses AI. Check for mistakes.
LPFloat cell_start_x = std::get<1>(chunk) + ((i / cells_per_dim_) % cells_per_dim_) * cell_size_;
LPFloat cell_start_y = std::get<2>(chunk) + (i % cells_per_dim_) * cell_size_;
LPFloat cell_start_z = std::get<3>(chunk) + (i / (cells_per_dim_ * cells_per_dim_)) * cell_size_;
Expand Down
5 changes: 4 additions & 1 deletion kagen/generators/geometric/rgg/rgg_2d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "kagen/generators/geometric/rgg/rgg_2d.h"

#include <algorithm>

#include "kagen/tools/geometry.h"

namespace kagen {
Expand Down Expand Up @@ -152,7 +154,8 @@ void RGG2D::GenerateCells(const SInt chunk_id) {
for (SInt i = 0; i < cells_per_chunk_; ++i) {
seed = config_.seed + chunk_id * cells_per_chunk_ + i + total_chunks_ * cells_per_chunk_;
SInt h = sampling::Spooky::hash(seed);
SInt cell_vertices = rng_.GenerateBinomial(h, n, cell_area / total_area);
// due to potential floating point inaccuracies clamp probability
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
LPFloat cell_start_x = std::get<1>(chunk) + (i / cells_per_dim_) * cell_size_;
LPFloat cell_start_y = std::get<2>(chunk) + (i % cells_per_dim_) * cell_size_;

Expand Down
5 changes: 4 additions & 1 deletion kagen/generators/geometric/rgg/rgg_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "kagen/generators/geometric/rgg/rgg_3d.h"

#include <algorithm>

namespace kagen {
RGG3D::RGG3D(const PGeneratorConfig& config, const PEID rank, const PEID size) : Geometric3D(config, rank, size) {
// Chunk variables
Expand Down Expand Up @@ -176,7 +178,8 @@ void RGG3D::GenerateCells(const SInt chunk_id) {
for (SInt i = 0; i < cells_per_chunk_; ++i) {
seed = config_.seed + chunk_id * cells_per_chunk_ + i + total_chunks_ * cells_per_chunk_;
SInt h = sampling::Spooky::hash(seed);
SInt cell_vertices = rng_.GenerateBinomial(h, n, cell_area / total_area);
// due to potential floating point inaccuracies clamp probability
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[nitpick] For type safety and consistency, consider using explicit type casting for the clamp bounds. Since cell_area and total_area are of type LPFloat, use std::clamp(cell_area / total_area, LPFloat{0.0}, LPFloat{1.0}) to match the pattern used in hyperbolic.cpp with Double{0.0} and Double{1.0}.

Suggested change
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, 0.0, 1.0));
SInt cell_vertices = rng_.GenerateBinomial(h, n, std::clamp(cell_area / total_area, LPFloat{0.0}, LPFloat{1.0}));

Copilot uses AI. Check for mistakes.
LPFloat cell_start_x = std::get<1>(chunk) + ((i / cells_per_dim_) % cells_per_dim_) * cell_size_;
LPFloat cell_start_y = std::get<2>(chunk) + (i % cells_per_dim_) * cell_size_;
LPFloat cell_start_z = std::get<3>(chunk) + (i / (cells_per_dim_ * cells_per_dim_)) * cell_size_;
Expand Down
6 changes: 4 additions & 2 deletions kagen/generators/hyperbolic/hyperbolic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "kagen/tools/postprocessor.h"

#include <iostream>
#include <algorithm>

namespace kagen {
PGeneratorConfig
Expand Down Expand Up @@ -188,7 +189,8 @@ void Hyperbolic<Double>::ComputeAnnuli(const SInt chunk_id) {

// Variate
SInt h = sampling::Spooky::hash(config_.seed + total_annuli_ * config_.k + chunk_id * total_annuli_ + i);
SInt n_annulus = rng_.GenerateBinomial(h, n, ring_area / total_area);
// due to potential floating point inaccuracies clamp probability
SInt n_annulus = rng_.GenerateBinomial(h, n, std::clamp(ring_area / total_area, Double {0.0}, Double{1.0}));

// Push annuli_
annuli_[ComputeGlobalChunkId(i - 1, chunk_id)] = std::make_tuple(n_annulus, min_r, max_r, false, offset);
Expand Down Expand Up @@ -294,7 +296,7 @@ void Hyperbolic<Double>::GenerateCells(const SInt annulus_id, SInt chunk_id) {
if (!clique)
seed = config_.seed + annulus_id * config_.k + chunk_id + i + config_.n;
SInt h = sampling::Spooky::hash(seed);
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Missing explanatory comment. For consistency with line 192-193 and the other files in this PR, add a comment here: // due to potential floating point inaccuracies clamp probability

Suggested change
SInt h = sampling::Spooky::hash(seed);
SInt h = sampling::Spooky::hash(seed);
// due to potential floating point inaccuracies clamp probability

Copilot uses AI. Check for mistakes.
SInt n_cell = rng_.GenerateBinomial(h, n, grid_phi / total_phi);
SInt n_cell = rng_.GenerateBinomial(h, n, std::clamp(grid_phi / total_phi, Double{0.0}, Double{1.0}));
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Inconsistent spacing in type construction. Line 193 uses Double {0.0} (with space), but this line uses Double{0.0} (without space). For consistency, use the same spacing convention as line 193.

Suggested change
SInt n_cell = rng_.GenerateBinomial(h, n, std::clamp(grid_phi / total_phi, Double{0.0}, Double{1.0}));
SInt n_cell = rng_.GenerateBinomial(h, n, std::clamp(grid_phi / total_phi, Double {0.0}, Double {1.0}));

Copilot uses AI. Check for mistakes.

SInt global_cell_id = ComputeGlobalCellId(annulus_id, chunk_id, i);
cells_[global_cell_id] =
Expand Down