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

Clean up training code #809

Merged
merged 2 commits into from
Jun 30, 2017
Merged

Clean up training code #809

merged 2 commits into from
Jun 30, 2017

Conversation

gunthercox
Copy link
Owner

This makes the following changes to the code in the training module:

  • Variables have been renamed to be more accurate and descriptive.
  • The ChatterBotCorpus trainer is no longer dependent on the ListTrainer.
  • The entire statement history for a conversation is no longer stored (only the last statement is ever needed).

)

statement_history.append(statement)
previous_statement_text.append(statement.text)
Copy link
Owner Author

Choose a reason for hiding this comment

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

Should have been a equal sign 🥇

for pair in data:
trainer.train(pair)
for corpus_path in corpus_paths:

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gunthercox Master, How about list comprehension, iterating each list will take long time if we train with huge data

Copy link
Owner Author

Choose a reason for hiding this comment

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

List comprehension is always an option to improve performance. I do have concerns about readability though as the format of list comprehension is often difficult to understand at a quick glance.

@gunthercox gunthercox merged commit 6045dc7 into master Jun 30, 2017
@gunthercox gunthercox deleted the train branch July 19, 2017 01:52
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.

2 participants