Skip to content

feat(path): Add probabilistic edge generation with p parameter #90

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

Open
niklas-uhl wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(path): Add probabilistic edge generation with p parameter #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e74775b
feat(path): Add probabilistic edge generation with p parameter
niklas-uhl Feb 3, 2026
ffd57e9
feat(path): Set p explicitly to 1 in tests.
niklas-uhl Feb 3, 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
2 changes: 1 addition & 1 deletion app/KaGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ This is mostly useful for experimental graph generators or when using KaGen to l

auto* params = cmd->add_option_group("Parameters");
add_option_n(params);
params->require_option(1);
add_option_p(params);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The CLI option -p/--prob is added for path-directed, but the help/default shown to users will still reflect config.p’s initial value (0.0), while runtime behavior normalizes it to 1.0. Consider setting the CLI default explicitly (e.g., default value 1.0 for this subcommand) so --help matches the documented “defaulting p=1.0” behavior and so p=0 can remain a valid, explicit input.

Suggested change
add_option_p(params);
params
->add_option(
"-p,--prob", config.p,
"Edge probability p for the directed path (defaults to p=1.0 for this generator).")
->default_val(1.0);

Copilot uses AI. Check for mistakes.
params->silent();
}

Expand Down
16 changes: 13 additions & 3 deletions kagen/generators/path/path_directed.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "kagen/generators/path/path_directed.h"
#include "kagen/sampling/hash.hpp"

#ifdef KAGEN_XXHASH_FOUND
#include "kagen/tools/random_permutation.h"
Expand All @@ -17,14 +18,19 @@ PGeneratorConfig PathDirectedFactory::NormalizeParameters(PGeneratorConfig confi
throw ConfigurationError("path permutation requires xxHash, but build was configured without xxHash");
}
#endif // KAGEN_XXHASH_FOUND

if (config.p == 0.0) {
config.p = 1.0;
}
if (config.p > 1.0) {
throw ConfigurationError("edge probability p must be in [0, 1]");
}
Comment on lines +21 to +26
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

NormalizeParameters() forces p==0.0 to 1.0 and only checks the upper bound. This makes it impossible to request a true p=0 (even though the error message claims [0, 1]) and allows negative/NaN values to flow into std::binomial_distribution, which has undefined behavior for out-of-range probabilities. Consider (a) validating p with std::isfinite(p) and 0.0 <= p && p <= 1.0, and (b) setting the default to 1.0 at the CLI layer (or via a dedicated “unset” sentinel) instead of rewriting p==0.0.

Copilot uses AI. Check for mistakes.
return config;
}

PathDirected::PathDirected(const PGeneratorConfig& config, const PEID rank, const PEID size)
: config_(config),
rank_(rank),
size_(size) {}
size_(size), rng_(config) {}

void PathDirected::GenerateEdgeList() {
if (config_.n <= 1) {
Expand Down Expand Up @@ -59,7 +65,11 @@ void PathDirected::GenerateEdgeList() {
}();

if (is_valid) {
PushEdge(i, j);
SInt edge_seed = std::min(i, j) *config_.n + std::max(i, j);
SInt h = sampling::Spooky::hash(config_.seed + edge_seed);
if (rng_.GenerateBinomial(h, 1, config_.p)) {
PushEdge(i, j);
}
Comment on lines 67 to +72
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

GenerateEdgeList() uses config_.p directly when deciding whether to emit each edge. When PathDirected is constructed directly (as in these tests), NormalizeParameters() is not invoked, so the default config.p=0.0 currently means “generate no edges” rather than the PR’s stated default of p=1.0. To preserve the intended default consistently, consider deriving an internal edge_probability_ (e.g., p==0 ? 1 : p) and validating it in the constructor/GenerateEdgeList instead of relying on factory normalization.

Copilot uses AI. Check for mistakes.
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions kagen/generators/path/path_directed.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "kagen/context.h"
#include "kagen/generators/generator.h"
#include "kagen/kagen.h"
#include "kagen/tools/rng_wrapper.h"

namespace kagen {
class PathDirectedFactory : public GeneratorFactory {
Expand All @@ -27,5 +28,7 @@ class PathDirected : public virtual Generator, private EdgeListOnlyGenerator {

PEID rank_;
PEID size_;

RNGWrapper<> rng_;
};
} // namespace kagen
2 changes: 2 additions & 0 deletions tests/path/general_path_generator_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ TEST_P(PathGeneratorTestFixture, path_generation_without_permutation) {

PGeneratorConfig config;
config.n = GetParam();
config.p = 1.0;

PathDirected generator(config, rank, size);
Comment on lines 77 to 81
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Tests were updated to set config.p = 1.0, but the new probabilistic behavior itself isn’t exercised. To cover the feature, add assertions for at least p=0.0 (no edges) and one intermediate probability with a fixed seed (deterministic subset of edges), so regressions in the hashing/sampling logic are caught.

Copilot uses AI. Check for mistakes.
generator.Generate(GraphRepresentation::EDGE_LIST);
Expand All @@ -94,6 +95,7 @@ TEST_P(PathGeneratorTestFixture, path_generation_with_permutation) {

PGeneratorConfig config;
config.n = GetParam();
config.p = 1.0;
config.permute = true;

PathDirected generator(config, rank, size);
Expand Down
Loading