fix: distance_range calculation #626
Conversation
maqi
force-pushed
the
regression_confirmation
branch
from
2052b95
to
da80465
Compare
| @@ -57,10 +57,12 @@ impl Node { | |||
| } | |||
|
|
|||
| // set the distance range used for pruning | |||
| let distance_range = match our_close_group.get(CLOSE_GROUP_SIZE) { | |||
| // our_close_group now only have CLOSE_GROUP_SIZE entries, | |||
| // which makes `our_close_group.get(CLOSE_GROUP_SIZE)` return None always. | |||
There was a problem hiding this comment.
I'm not sure I follow why this is always None?
There was a problem hiding this comment.
our_close_group now only contains 8th entries,
but .get(CLOSE_GROUP_SIZE) is asking for the entry with index 8.
As the vec's indexing is from 0, hence it's actually looking from 9th entry, which is None
There was a problem hiding this comment.
Ah, i'd perhaps remove this line // which makes `our_close_group.get(CLOSE_GROUP_SIZE)` return None always. as it's a bit confusing (as we dont do it?)
We can just not perhaps // our_close_group now only have CLOSE_GROUP_SIZE entries, so we grab the last entry
maybe last and farthest if they are sorted?
There was a problem hiding this comment.
Ah, that comment is mainly explain what's the issue of previous, i.e. our_close_group.get(CLOSE_GROUP_SIZE) .
Not about the current approach (using last()).
maqi
force-pushed
the
regression_confirmation
branch
from
da80465
to
2c558d9
Compare
|
already in main |
Description
Summary generated by Reviewpad on 09 Aug 23 11:24 UTC
This pull request makes a minor correction to the
.github/workflows/merge.ymlfile. It changes the name of a test from "execute the dbc spend test" to "execute the dbc spend tests".