Skip to content
This repository has been archived by the owner on Jun 13, 2020. It is now read-only.

Fix problematic case with opts==None and therefore no opts.use_dropout. #3

Closed
wants to merge 2 commits into from

Conversation

benelot
Copy link

@benelot benelot commented Jan 2, 2017

I found some problem(s) while reading through your code and running it with different settings. I think it would be helpful if I contributed some fix(es) back while I do this. For now, there is only one fix in the request but maybe there are more to come.

@@ -1,5 +1,5 @@
#!/usr/bin/env python
import event_pb2
from tensorflow.core.util import event_pb2
Copy link
Owner

Choose a reason for hiding this comment

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

this is actually importing a different proto; i'm wanting to use the event.proto at the root of this repro.

it's a bit hacky as it is now; this code requires you run protoc event.proto --python_out=. (as mentioned in the README.md) before the event_pb2.py is created. what i should do is try the import and, if it fails, give a warning that this code needs to be run (either that or make it a proper part of some kind of setup.py init...

Copy link
Owner

@matpalm matpalm left a comment

Choose a reason for hiding this comment

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

thanks for the pull request. see note re: the event_pb, i should have made this code more robust and explicit about the requirement to compile the proto file sorry...

@benelot
Copy link
Author

benelot commented Jan 3, 2017

Oh ok, I will check later if I can add this quickly to the setup.py if you do not have time to fix it quickly. It seems to work with the other one as well btw, but I did not look into the details, I just ran it and it trained. So I can not say if it actually works or if the code just "runs".

@matpalm
Copy link
Owner

matpalm commented Jan 3, 2017

yeah the import would have worked but then it would have failed if you'd being using any type of event logging (i.e. --event-log=...) i use the logging of runs for debugging, prepopulating replay memories etc, but it's not a requirement, you can train / run without it (which is why i think i'll move the import to just before it's being used) thanks for the sanity checks!

@benelot
Copy link
Author

benelot commented Jan 4, 2017

Thanks, nice to know!

@matpalm matpalm closed this Jun 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants