Calculate implied edge weights from joint probabilities#129
Conversation
oscarhiggott
left a comment
There was a problem hiding this comment.
Mostly looks good, however the weights should be kept as floats while they are in the UserGraph and only converted to ints after they are rescaled during the conversion to a MatchingGraph
| pm::weight_int pm::convert_probability_to_weight(double p) { | ||
| return std::log((1 - p) / p); | ||
| } |
There was a problem hiding this comment.
The weights should just be stored as floats, not ints, at this stage. The weights need to be rescaled before being turned into integers, but this is only done when the UserGraph is converted to the MatchingGraph
There was a problem hiding this comment.
Fixed (this code was actually removed) by changes from an upstream PR
|
|
||
| TEST(ConvertProbabilityToWeight, PositiveResultIsTruncated) { | ||
| const double p = 0.1; | ||
| const pm::weight_int expected_weight = 2; // 2.197... is truncated to 2 | ||
| EXPECT_EQ(pm::convert_probability_to_weight(p), expected_weight); | ||
| } | ||
|
|
||
| TEST(ConvertProbabilityToWeight, SmallPositiveResultTruncatesToZero) { | ||
| const double p = 0.4; | ||
| const pm::weight_int expected_weight = 0; | ||
| EXPECT_EQ(pm::convert_probability_to_weight(p), expected_weight); | ||
| } |
There was a problem hiding this comment.
This is something we don't want to happen (should rescale before converting to int)
There was a problem hiding this comment.
Fixed (this code was actually removed) by changes from an upstream PR
| struct ImpliedWeightUnconverted { | ||
| size_t node1; | ||
| size_t node2; | ||
| weight_int new_weight; |
There was a problem hiding this comment.
It should be:
double new_weight;
instead of
weight_int new_weight;
There was a problem hiding this comment.
Fixed by changes from an upstream PR
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
…ings Update pybindings to handle correlations
…logic Fix search graph logic to not depend on the implied weights inside the graph flooder.
Pipe reweighting logic all the way to the Pymatching binary CLI
…ethod Implement reweight logic for the search and matching graphs
…ghts Finish ImpliedWeight conversion (with pointers to weights to be reweighted).
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
Co-authored-by: oscarhiggott <29460323+oscarhiggott@users.noreply.github.com>
…ts-to-matching-graph Populate unconverted implied weights during matching graph creation
…edges Populate implied edge weights in edges in the User Graph
…ts-in-matching-graph Only discretize weights when converting to MatchingGraphs
…d_weight_validity Check implied weight validity when computing normalizing constant
…ts-in-matching-graph Merge validity check changes into reweighting fixes
Add validity checks and no more discretized edges into u/smadhuk/calculate-edge-weights
…mber Make previously free edges to implied weights unconverted variable a class member
Add in edge weight calculation when creating a
user_graphwith correlations enabled. Populating edges with their implied weight is aTODO.-- Performance before this PR --
-- Performance after this PR --