Skip to content

[BugFix, Feature] Fix KeyError in NXGraphCompiler and extending the class of embeddable graphs to unit disk graphs#116

Merged
RolandMacDoland merged 7 commits into
pasqal-io:mainfrom
lmoroml:fix_networkx
May 14, 2025
Merged

[BugFix, Feature] Fix KeyError in NXGraphCompiler and extending the class of embeddable graphs to unit disk graphs#116
RolandMacDoland merged 7 commits into
pasqal-io:mainfrom
lmoroml:fix_networkx

Conversation

@lmoroml
Copy link
Copy Markdown
Contributor

@lmoroml lmoroml commented Apr 28, 2025

This pull request introduces the following improvements and bug fixes:

  1. Fix for KeyError in NXGraphCompiler: an issue was identified where a KeyError: "x" occurred when a NetworkX graph was passed to the NXGraphCompiler. This error was caused by a missing check for the existence of the "x" attribute in the graph. The fix involves adding a validation step to ensure that the "x" attribute is present before it is accessed.

  2. New Method: is_unit_disk_graph: a new method, is_unit_disk_graph(), has been added to the BaseGraph class. This method provides a straightforward way to check whether a graph is a unit disk graph, rather than a disk graph.

  3. Update to is_embeddable Method: the is_embeddable() method has been modified to use is_unit_disk_graph() rather than the previous is_disk_graph() method. The change was made because the old implementation, which used a radius of min_atom_distance + epsilon, while correct in theory, unnecessarily limited the class of embeddable graphs. By restricting embeddability to only those unit disk graphs with edges between min_atom_edge_distance and min_atom_edge_distance + epsilon, the method was excluding graphs that should have been considered embeddable. The new approach broadens the criteria allowing to embed unit disk graphs.

@RolandMacDoland RolandMacDoland changed the title Fix KeyError in NXGraphCompiler and extending the class of embeddable graphs to unit disk graphs [BugFix, Feature] Fix KeyError in NXGraphCompiler and extending the class of embeddable graphs to unit disk graphs Apr 29, 2025
@RolandMacDoland RolandMacDoland added bug Something isn't working feature labels Apr 29, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 29, 2025

Codecov Report

Attention: Patch coverage is 2.77778% with 35 lines in your changes missing coverage. Please review.

Project coverage is 38.81%. Comparing base (ddc8cee) to head (2f10e4d).

Files with missing lines Patch % Lines
tests/test_graphs.py 0.00% 20 Missing ⚠️
qek/data/graphs.py 6.25% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   39.91%   38.81%   -1.11%     
==========================================
  Files          17       17              
  Lines        1175     1211      +36     
  Branches      136      140       +4     
==========================================
+ Hits          469      470       +1     
- Misses        677      712      +35     
  Partials       29       29              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Yoric
Copy link
Copy Markdown
Contributor

Yoric commented Apr 29, 2025

Thanks for the PR, I'll try and review it by tomorrow.

Copy link
Copy Markdown
Contributor

@RolandMacDoland RolandMacDoland left a comment

Choose a reason for hiding this comment

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

Thanks a lot @lmoroml nice work indeed :) LGTM. Minor comments.

Comment thread qek/data/graphs.py Outdated
Comment thread tests/test_graphs.py
Comment thread tests/test_graphs.py Outdated
lmoroml and others added 2 commits May 5, 2025 13:24
Co-authored-by: RolandMacDoland <9250798+RolandMacDoland@users.noreply.github.com>
Co-authored-by: RolandMacDoland <9250798+RolandMacDoland@users.noreply.github.com>
@Yoric
Copy link
Copy Markdown
Contributor

Yoric commented May 12, 2025

My apologies for the delay, I've been sick :/

Looks good to me, thanks for the PR!

@RolandMacDoland RolandMacDoland merged commit 78a9daf into pasqal-io:main May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants