Skip to content
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

fix: correct best_instance_index in batch.select_instance #29

Merged
merged 1 commit into from
Jan 14, 2019
Merged

fix: correct best_instance_index in batch.select_instance #29

merged 1 commit into from
Jan 14, 2019

Conversation

zhangyu94
Copy link
Contributor

modAL/modAL/batch.py

Lines 104 to 106 in 452898f

best_instance_index = np.argmax(scores)
mask[best_instance_index] = 0
return best_instance_index, X_pool[best_instance_index].reshape(1, -1), mask

Hi!

This pull request addresses the bug that the computed best_instance_index in batch.select_instance is the index of the best instance in X_pool[mask] instead of the index in X_pool.

For this reason, the returned X_pool[best_instance_index].reshape(1, -1) and mask (updated with mask[best_instance_index] = 0) are both incorrect.

@codecov-io
Copy link

Codecov Report

Merging #29 into dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #29      +/-   ##
==========================================
+ Coverage   98.05%   98.05%   +<.01%     
==========================================
  Files          30       30              
  Lines        1594     1597       +3     
==========================================
+ Hits         1563     1566       +3     
  Misses         31       31
Impacted Files Coverage Δ
modAL/batch.py 97.72% <100%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 452898f...f8778f5. Read the comment docs.

@cosmic-cortex cosmic-cortex merged commit f8778f5 into modAL-python:dev Jan 14, 2019
@cosmic-cortex
Copy link
Member

Thanks! I have reviewed the code, the original interpretation was indeed incorrect in this case.

@cosmic-cortex cosmic-cortex added this to Implemented in dev branch in modAL Jan 21, 2019
@cosmic-cortex cosmic-cortex moved this from Implemented in dev branch to Merged to master branch in modAL May 10, 2019
@cosmic-cortex cosmic-cortex removed this from Merged to master branch in modAL Nov 11, 2019
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.

None yet

3 participants