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 problem with excluding training ops #835

Merged
merged 1 commit into from
Jul 12, 2017
Merged

Fix problem with excluding training ops #835

merged 1 commit into from
Jul 12, 2017

Conversation

exelents
Copy link
Contributor

@exelents exelents commented Jul 12, 2017

I've found a problem on a line 240 of tflearn/helpers/trainer.py where i've made changes.
The problem is that a cycle, where program removes excluded training operations from actual ones, it doesn't remove all training operations that must be excluded. I'll try to explain this cycle on similar task of removing numbers from an array. Let's see:

a = [2, 2, 2, 2, 2, 2, 2 , 2, 2, 2 ,2 ,2 , 2]

print(a)

for t in a:
    if t == 2:
        a.remove(t)
    
print(a)

This is a similar cycle that removes number 2 from array instead of excluded trainops. You think that it'll remove all 2 and resulting array a will be empty. But if you run this code you'll see, that it six numbers 2 in your array left!
It's happen so, because, ii seems to me, Python's iterator have some attachements to index of element instead of element itself. How it's hapenning:

first iteration:
elements | a b c d
indexes  | 0 1 2 3
you here | ^

cycle takes zero element from an array. Element 'a' removed
second iteration

elements | b c d
old ind. | 1 2 3
indexes  | 0 1 2
you here |   ^

iterator takes element with index 1, that have a sense if array is still similar. But as first item was removed, indexes of elements has changed, and now element 1 is a an element 2 from previous iteration.
That way your cycle doesn't always pass over all items, while there are training operations to exclude. I've found this error when tried to use on my graph three different trainops, that should run separately in different training steps.

Copy link
Member

@aymericdamien aymericdamien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thanks for fixing it :)

@aymericdamien aymericdamien merged commit b52030a into tflearn:master Jul 12, 2017
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

2 participants