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

[autopilot] Weighted combined attachment heuristic #2405

Merged
merged 3 commits into from Jan 23, 2019

Conversation

Projects
None yet
3 participants
@halseth
Copy link
Collaborator

halseth commented Jan 3, 2019

Follow-up to #2341.

We make a new autopilot heuristic WeightedCombAttachment that combines scores from multiple sub-heuristics into one that can be used by the agent.

To make this possible we make sure the NodeScore API always returns scores in the range 0.0-1.0.

For now we only use the existing heuristic with a weight of 1.0, but this will change in the follow-up PRs.

Builds on #2341

@halseth halseth changed the title [autopilot] Weighted heuristics [autopilot] Weighted combined attachment heuristics Jan 3, 2019

@halseth halseth force-pushed the halseth:autopilot-weighted-heuristics-follow-up branch from 6d211d9 to 521f27e Jan 3, 2019

@halseth halseth changed the title [autopilot] Weighted combined attachment heuristics [autopilot] Weighted combined attachment heuristic Jan 3, 2019

@halseth halseth referenced this pull request Jan 3, 2019

Merged

Bos score enabled Autopilot #2212

6 of 6 tasks complete
@Roasbeef

This comment has been minimized.

Copy link
Member

Roasbeef commented Jan 9, 2019

Needs rebase!

score := float64(nodeChans) / float64(graphChans)
// channels in the graph, scaled such that the highest-degree
// node will be given a score of 1.0.
score := float64(nodeChans) / float64(maxChans)

This comment has been minimized.

@Roasbeef

Roasbeef Jan 9, 2019

Member

This change significantly alters the existing algorithm. I don't see why this change is needed, since without this, the score is still in the range of 0 to 1.

This comment has been minimized.

@halseth

halseth Jan 9, 2019

Author Collaborator

The idea here is that it will make it easier to combine multiple heuristics. I ran into issues when attempting to combine the scores returned from the existing algorithm with the bos scores.

Before this change the prefattach scores would look like [A: 0.002, B: 0.03, C: 0.0024, ...], i.e. very small scores for most nodes. The bos scores would be binary in the sense that nodes not in the list would implicitly be given a zero-score: [A: 1.0, C: 1.0, ...].

This made it very hard to find the correct weighting if I wanted to combine them. For instance if I wanted to consider the two heuristics equally, I would give them both a weight of 0.5. However, for the example above this would result in scores like [A: 0.5001, B: 0.015, C: 0.5012]. Since the scales of the scores would be vastly different, to find the right weighting was hard. Also nodes considered good by the prefattach algorithm would almost never get picked, since they wasn't found in the score list.

To your first point: this shouldn't change the algorithm, only the scale of the returned scores. Consider the old version for score given to node i:

score_i := nodeChans_i / graphChans

This change scales all these scores by finding the maximum score and multiplying all by (1.0 / score_max):

score_max := maxChans / graphChans

score_i = score_i * (1.0 / score_max)
             = (nodeChans_i / graphChans) * (1.0 / (maxChans / graphChans))
             = (nodeChans_i / graphChans) * (graphChans / maxChans)
             = nodeChans_i / maxChans

With this change, the "best" node from the PoV of the prefattach will be given a score of 1.0. A equal weighting of the two heuristic will then give it the expected score of 1.0 if it's in the score list, and 0.5‚ if not.

This comment has been minimized.

@Roasbeef

Roasbeef Jan 19, 2019

Member

Also nodes considered good by the prefattach algorithm would almost never get picked, since they wasn't found in the score list.

That isn't necessarily a bad thing, it could be node that seem well connected, but in reality aren't able to properly route payments.

Show resolved Hide resolved autopilot/combinedattach.go

@halseth halseth force-pushed the halseth:autopilot-weighted-heuristics-follow-up branch 2 times, most recently from e98c1ed to a2076ff Jan 9, 2019

Show resolved Hide resolved autopilot/combinedattach.go
Show resolved Hide resolved autopilot/combinedattach.go
Show resolved Hide resolved autopilot/combinedattach.go Outdated

@Roasbeef Roasbeef added this to the 0.6 milestone Jan 16, 2019

@halseth halseth force-pushed the halseth:autopilot-weighted-heuristics-follow-up branch from a2076ff to d87f5b5 Jan 16, 2019

@halseth

This comment has been minimized.

Copy link
Collaborator Author

halseth commented Jan 16, 2019

Comments addressed. PTAL @Roasbeef @cfromknecht

@Roasbeef
Copy link
Member

Roasbeef left a comment

LGTM 💥

Will be able to do more testing with this to feel out the changes to the pref attachment heuristic once the dry run PR is merged.

Show resolved Hide resolved autopilot/combinedattach.go

halseth added some commits Jan 9, 2019

autopilot/interface+prefattach: scale node scores to range [0.0, 1.0]
To prepare for combinning scores from multiple heuristics, we require the
scores returned from the NodeSores API to be in the range [0.0, 1.0].

The prefAttach heuristic is altered to scale the returned scores such
that the most connected node in the grpah is given a score of 1.0.
autopilot: add WeightedCombAttachment
This commit defines a new heuristic WeightedCombAttachment that takes a
set of sub-heuristics, and produces a final node score by querying the
sub-heuristics and combining the scores from them according to their
weights.

This way it will look like a regular, single heuristic to the autopilot
agemnt, but can be a more complex combination of several.
lnd+pilot: use WeightedCombAttachment
We make the default autopilot agent use the WeightedCombAttachment.
Currently it uses only one sub-heuristic, prefAttachment.

@halseth halseth force-pushed the halseth:autopilot-weighted-heuristics-follow-up branch from d87f5b5 to f48c8f9 Jan 21, 2019

@Roasbeef Roasbeef merged commit cebc4d8 into lightningnetwork:master Jan 23, 2019

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.06%) to 56.252%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment