Skip to content

Keras models support #7

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

Merged

Conversation

typhoonzero
Copy link
Collaborator

@typhoonzero typhoonzero commented May 22, 2019

Also removed python3.8 test because of this error: https://travis-ci.com/sql-machine-learning/models/jobs/202072631

Related work: sql-machine-learning/sqlflow#424

tests/base.py Outdated
self.setUp()
train_input_fn(self.features, self.label)

print('Calling BaseTest:testCommon')
Copy link
Contributor

@tonyyang-svail tonyyang-svail May 22, 2019

Choose a reason for hiding this comment

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

Should we also test self.model.fit and self.model.predict here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.


def default_training_epochs(self):
"""Default training epochs. Used in model.fit."""
return 2
Copy link
Contributor

@tonyyang-svail tonyyang-svail May 22, 2019

Choose a reason for hiding this comment

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

2 might be too small. Change to 50?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tested 2 is enough for iris demo case. Change to 50 maybe take too long to train

@tonyyang-svail tonyyang-svail self-requested a review May 22, 2019 14:58
wangkuiyi
wangkuiyi previously approved these changes May 23, 2019
Copy link
Contributor

@tonyyang-svail tonyyang-svail left a comment

Choose a reason for hiding this comment

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

LGTM!

@typhoonzero typhoonzero merged commit 77d1b58 into sql-machine-learning:develop May 23, 2019
@typhoonzero typhoonzero deleted the keras_models_support branch May 23, 2019 02:44
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