Skip to content

Conversation

@cclauss
Copy link
Contributor

@cclauss cclauss commented Jan 30, 2018

First reported in #1981 ...

is_training is an undefined name inside the function visit_count_fc() because the function takes 5 parameters and none of them is called is_training. Undefined names might raise NameError at runtime.

flake8 testing of https://github.com/tensorflow/models

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./research/cognitive_mapping_and_planning/tfcode/vision_baseline_lstm.py:152:21: F821 undefined name 'is_training'
        is_training=is_training)
                    ^

In the tensorflow/models repo, there is only one caller of the function visit_count_fc() and that caller passes 6 parameters and the last one is called is_training.

The proposed fix is to add is_training as the sixth parameter of visit_count_fc() to match both the caller and the internals of the function and thereby eliminate the undefined name.

@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@cclauss
Copy link
Contributor Author

cclauss commented Feb 2, 2018

@nealwu @karmel Could we please get a reviewer assigned? Any comments on https://github.com/tensorflow/models/pulls/cclauss

@karmel
Copy link
Contributor

karmel commented Feb 3, 2018

Thanks, @cclauss . Research models are the responsibility of the individual researchers listed in the owners file. For small changes like this, I can do my best, but can you provide evidence that you have run the models and they perform/converge as expected? Otherwise, I leave to @s-gupta to review as owner.

@cclauss
Copy link
Contributor Author

cclauss commented Feb 5, 2018

@s-gupta Can we please get your review on these changes?

@cclauss
Copy link
Contributor Author

cclauss commented Feb 23, 2018

bump on this PR. This is the last of the Python 2 undefined names in the models codebase...

flake8 testing of https://github.com/tensorflow/models on Python 2.7.14

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./research/cognitive_mapping_and_planning/tfcode/vision_baseline_lstm.py:152:21: F821 undefined name 'is_training'
        is_training=is_training)
                    ^
1     F821 undefined name 'is_training'

@karmel karmel added the stat:awaiting maintainer Waiting on input from the maintainer label Feb 23, 2018
@cclauss
Copy link
Contributor Author

cclauss commented Feb 28, 2018

@karmel Is there an owner that has the commit bit that can review this one?

@karmel
Copy link
Contributor

karmel commented Feb 28, 2018

@cclauss If you can confirm that the model still runs as intended, I can merge, and @s-gupta or others can create new issues to roll back if necessary. But, please run the model to confirm that it still works.

@cclauss cclauss closed this Jun 25, 2018
@cclauss cclauss deleted the undefined-name-is_training branch June 25, 2018 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes stat:awaiting maintainer Waiting on input from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants