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

chore(lib): warn about overflow and add Rust coverage #116

Merged
merged 6 commits into from
Sep 18, 2024
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
2 changes: 0 additions & 2 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ jobs:

- name: Install Python
uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install pre-commit
run: pip install pre-commit
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ jobs:

- name: Install Python
uses: actions/setup-python@v5
with:
python-version: '3.10'

- name: Build wheels
uses: PyO3/maturin-action@v1
Expand Down Expand Up @@ -57,7 +55,6 @@ jobs:
- name: Install Python
uses: actions/setup-python@v5
with:
python-version: '3.11'
architecture: ${{ matrix.target }}

- name: Build wheels
Expand Down
34 changes: 30 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v4

- uses: actions/setup-python@v5
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.pyversion }}

Expand All @@ -82,6 +83,34 @@ jobs:
- name: Run Cargo test
run: cargo test

cargo-tarpaulin:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup Python
uses: actions/setup-python@v5

- name: Install NumPy
run: pip install numpy

- name: Install Rust
uses: dtolnay/rust-toolchain@stable

- uses: taiki-e/install-action@v2
with:
tool: cargo-tarpaulin@0.31.2

- name: Generate code coverage
run: cargo tarpaulin --verbose --timeout 120 --out xml

- name: Upload to codecov.io
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.DIFFERT_CODECOV_TOKEN }}
fail_ci_if_error: true

python-benchmark:
runs-on: ubuntu-latest
permissions:
Expand All @@ -92,7 +121,6 @@ jobs:

- uses: actions/setup-python@v5
with:
python-version: '3.11'
cache: pip
cache-dependency-path: uv.lock

Expand Down Expand Up @@ -205,8 +233,6 @@ jobs:
uses: actions/checkout@v4

- uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install NumPy
run: pip install numpy
Expand Down
2 changes: 1 addition & 1 deletion .python-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.11.8
3.11
52 changes: 31 additions & 21 deletions differt-core/src/rt/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@
let from_in_graph = from < num_nodes;
let to_in_graph = to < num_nodes;

match (from_in_graph, to_in_graph) {
(match (from_in_graph, to_in_graph) {
(true, true) => {
//This solution was obtained thanks to user @ronno, see:
// https://math.stackexchange.com/a/4874894/1297520.
Expand All @@ -326,38 +326,48 @@
let num_nodes_minus_1 = num_nodes.saturating_sub(1);
if from != to {
num_nodes_minus_1
.saturating_pow(depth_minus_1)
.saturating_add_signed(if depth_minus_1 % 2 == 0 {
-1
} else {
1
.checked_pow(depth_minus_1)

Check warning on line 329 in differt-core/src/rt/graph.rs

View check run for this annotation

Codecov / codecov/patch

differt-core/src/rt/graph.rs#L329

Added line #L329 was not covered by tests
.and_then(|num| {
num.checked_add_signed(if depth_minus_1 % 2 == 0 {
-1
} else {
1
})
})
/ num_nodes
.map(|num| num / num_nodes)
} else {
num_nodes_minus_1
.saturating_pow(depth_minus_2)
.saturating_add_signed(if depth_minus_2 % 2 == 0 {
-1
} else {
1
.checked_pow(depth_minus_2)

Check warning on line 340 in differt-core/src/rt/graph.rs

View check run for this annotation

Codecov / codecov/patch

differt-core/src/rt/graph.rs#L340

Added line #L340 was not covered by tests
.and_then(|num| {
num.checked_add_signed(if depth_minus_2 % 2 == 0 {
-1

Check warning on line 343 in differt-core/src/rt/graph.rs

View check run for this annotation

Codecov / codecov/patch

differt-core/src/rt/graph.rs#L343

Added line #L343 was not covered by tests
} else {
1
})
})
* num_nodes_minus_1
/ num_nodes
.map(|num| (num / num_nodes) * num_nodes_minus_1)
}
},
(false, false) => {
// (num_nodes) * (num_nodes - 1)^(num_intermediate_nodes - 1)
(num_nodes).saturating_mul(
num_nodes
.saturating_sub(1)
.saturating_pow(num_intermediate_nodes.saturating_sub(1)),
)
num_nodes
.saturating_sub(1)
.checked_pow(num_intermediate_nodes.saturating_sub(1))

Check warning on line 355 in differt-core/src/rt/graph.rs

View check run for this annotation

Codecov / codecov/patch

differt-core/src/rt/graph.rs#L354-L355

Added lines #L354 - L355 were not covered by tests
.and_then(|num| num_nodes.checked_mul(num))
},
_ => {
// (num_nodes - 1)^num_intermediate_nodes
(num_nodes.saturating_sub(1)).saturating_pow(num_intermediate_nodes)
(num_nodes.saturating_sub(1)).checked_pow(num_intermediate_nodes)
},
}
})
.unwrap_or_else(|| {
log::warn!(
"OverflowError: overflow occurred when computing the total number of \
paths, defaulting to maximum value {}.",
usize::MAX

Check warning on line 367 in differt-core/src/rt/graph.rs

View check run for this annotation

Codecov / codecov/patch

differt-core/src/rt/graph.rs#L363-L367

Added lines #L363 - L367 were not covered by tests
);
usize::MAX

Check warning on line 369 in differt-core/src/rt/graph.rs

View check run for this annotation

Codecov / codecov/patch

differt-core/src/rt/graph.rs#L369

Added line #L369 was not covered by tests
})
},
};

Expand Down
48 changes: 47 additions & 1 deletion differt-core/tests/rt/test_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,27 @@ def test_all_keyword_only_parameters(self) -> None:
):
_ = graph.all_paths(0, 1, 0, True) # noqa: FBT003

@pytest.mark.parametrize(
("num_nodes", "depth"),
[
(15, 2),
(25, 3),
(11, 4),
],
)
def test_all_paths_count_from_complete_graph(
self,
num_nodes: int,
depth: int,
) -> None:
graph = CompleteGraph(num_nodes)
from_, to = num_nodes, num_nodes + 1
paths = graph.all_paths(from_, to, depth + 2, include_from_and_to=False)
num_paths = sum(1 for _ in paths)
assert num_paths == num_nodes * (num_nodes - 1) ** (depth - 1)
array = graph.all_paths_array(from_, to, depth + 2, include_from_and_to=False)
assert array.shape == (num_paths, depth)

@pytest.mark.parametrize(
("num_nodes", "depth"),
[
Expand All @@ -91,7 +112,7 @@ def test_all_keyword_only_parameters(self) -> None:
(10, 3),
],
)
def test_all_paths_count_from_complete_graph(
def test_all_paths_count_from_di_graph(
self,
num_nodes: int,
depth: int,
Expand All @@ -103,3 +124,28 @@ def test_all_paths_count_from_complete_graph(
assert num_paths == num_nodes * (num_nodes - 1) ** (depth - 1)
array = graph.all_paths_array(from_, to, depth + 2, include_from_and_to=False)
assert array.shape == (num_paths, depth)

@pytest.mark.parametrize(
("num_nodes", "depth"),
[
(10, 100),
(50_000, 10),
],
)
def test_all_paths_count_from_complete_graph_overflow(
self,
num_nodes: int,
depth: int,
caplog: pytest.LogCaptureFixture,
) -> None:
graph = CompleteGraph(num_nodes)
from_, to = num_nodes, num_nodes + 1
_ = graph.all_paths(from_, to, depth + 2, include_from_and_to=False)

for record in caplog.records:
assert record.levelname == "WARNING"

assert (
"OverflowError: overflow occurred when computing the total number of paths"
in caplog.text
)
Loading