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

training.py _check_loss_and_target_compatibility() fix crash if y is None #7071

Merged

Conversation

ahundt
Copy link
Contributor

@ahundt ahundt commented Jun 21, 2017

Don't access y param of model.fit() if it is None. This prevents a crash when model.fit(None,None, ...) is called and loss is provided to model.compile().

@stale
Copy link

stale bot commented Oct 24, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs, but feel free to re-open a closed issue if needed.

@stale stale bot added the stale label Oct 24, 2017
@ahundt
Copy link
Contributor Author

ahundt commented Nov 2, 2017

This one is still valid

@stale stale bot removed the stale label Nov 2, 2017
@ragulpr
Copy link

ragulpr commented Dec 2, 2017

+1 for this pull, it seems necessary for various hacks/workarounds for target/predicted compatibility checks and in order to reuse inputs in loss (ex autoencoders w. embedding layers)

Great work! Hope it's merged

@ahundt
Copy link
Contributor Author

ahundt commented Dec 2, 2017

@fchollet I know we passed on most of my other PRs once the target tensor parameter was added, but this one is trivially simple and worth merging.

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Sorry for not checking back on this earlier.

@fchollet fchollet merged commit 1f2e7a7 into keras-team:master Jan 20, 2018
@ahundt
Copy link
Contributor Author

ahundt commented Jan 20, 2018

thanks!

@ahundt ahundt deleted the check_loss_and_target_compatibility() branch January 20, 2018 02:36
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.

3 participants