Skip to content

Fix Cayley distance calculation by reordering search and assignment#443

Merged
osorensen merged 5 commits intomasterfrom
copilot/fix-lines-60-61-order
Jan 27, 2026
Merged

Fix Cayley distance calculation by reordering search and assignment#443
osorensen merged 5 commits intomasterfrom
copilot/fix-lines-60-61-order

Conversation

Copy link
Contributor

Copilot AI commented Jan 23, 2026

The Cayley distance implementation had lines 60-61 in the wrong order, causing the search operation to include the just-modified index.

Changes

  • src/distances.cpp: Swap lines 60-61 to search for swap positions before modifying the array

Technical Details

The bug:

double tmp1 = tmp2(i);
tmp2(i) = r2(i);                 // Modifies tmp2(i) first
uvec inds = find(tmp2 == r2(i)); // Search finds the just-set value at index i
tmp2.elem(inds).fill(tmp1);      // Incorrectly overwrites tmp2(i)

The fix:

double tmp1 = tmp2(i);
uvec inds = find(tmp2 == r2(i)); // Search before modification
tmp2(i) = r2(i);                 // Then modify
tmp2.elem(inds).fill(tmp1);

This ensures the algorithm finds the correct swap positions in the unmodified array.

Original prompt

This section details on the original issue you should resolve

<issue_title>Subtle bug?</issue_title>
<issue_description>I wonder if lines 60 and 61 should be swapped to avoid potential compiler issues.

double CayleyDistance::d(const vec& r1, const vec& r2) {
double distance{};
vec tmp2 = r1;
for(size_t i{}; i < r1.n_elem; ++i){
if(tmp2(i) != r2(i)) {
distance += 1;
double tmp1 = tmp2(i);
tmp2(i) = r2(i);
uvec inds = find(tmp2 == r2(i));
tmp2.elem(inds).fill(tmp1);
}
}
return distance;
}
</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: osorensen <21175639+osorensen@users.noreply.github.com>
@osorensen osorensen marked this pull request as ready for review January 23, 2026 10:09
Copilot AI and others added 2 commits January 23, 2026 10:10
Co-authored-by: osorensen <21175639+osorensen@users.noreply.github.com>
Co-authored-by: osorensen <21175639+osorensen@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix potential compiler issues by swapping lines 60 and 61 Fix Cayley distance calculation by reordering search and assignment Jan 23, 2026
Copilot AI requested a review from osorensen January 23, 2026 10:11
@osorensen osorensen requested a review from wleoncio January 23, 2026 10:21
@osorensen osorensen merged commit b1f045f into master Jan 27, 2026
9 checks passed
@osorensen osorensen deleted the copilot/fix-lines-60-61-order branch January 27, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subtle bug?

3 participants