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.

Sign up for GitHub

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

Jump to bottom

No database dependency during inference/serving #368

Merged
merged 12 commits into from
Feb 14, 2020
Merged

No database dependency during inference/serving #368

merged 12 commits into from
Feb 14, 2020

Conversation

HiromuHota
Copy link
Contributor

This will close #316 once it's finished.

@lukehsiao lukehsiao added this to the v0.8.0 milestone Dec 4, 2019
@lukehsiao lukehsiao added the enhancement New feature or request label Dec 4, 2019
@HiromuHota
Copy link
Contributor Author

Although it needs some code cleanup, it's now functioning.
As can be seen in https://github.com/HiromuHota/fonduer-mlflow/tree/fix/316, a database is not needed during inference.
I'll do some code cleaning and make it ready for review shortly.

@HiromuHota HiromuHota force-pushed the fix/316 branch 2 times, most recently from 2e7face to 38f8101 Compare January 22, 2020 18:40
@HiromuHota HiromuHota mentioned this pull request Jan 22, 2020
Merged
@HiromuHota
Copy link
Contributor Author

The number of commits was reduced to 18, but still a lot to review.
I'll be splitting them into smaller PRs so that it is easier to review.

This was referenced Jan 24, 2020
Merged
Merged
@HiromuHota HiromuHota marked this pull request as ready for review January 25, 2020 00:19
lukehsiao
lukehsiao suggested changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@lukehsiao lukehsiao left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Otherwise, LGTM. I'll take a bit of a closer pass as well.

src/fonduer/parser/parser.py Outdated Show resolved Hide resolved
@lukehsiao
Copy link
Contributor

I know this is unrelated to this specific PR, but I wonder if there is a way to eliminate the ~1k warnings we get in test_e2e on travis.

@lukehsiao lukehsiao added the clean-up Cleaning up the code or refactoring label Jan 27, 2020
@lukehsiao lukehsiao requested a review from senwu January 27, 2020 21:42
@HiromuHota
Copy link
Contributor Author

If I remember correctly, the ~1k warnings come from Snorkel where it uses deprecated APIs of PyTorch.

senwu
senwu approved these changes Feb 13, 2020
View reviewed changes
src/fonduer/candidates/candidates.py Outdated Show resolved Hide resolved
src/fonduer/candidates/candidates.py Outdated Show resolved Hide resolved
src/fonduer/candidates/candidates.py Show resolved Hide resolved
src/fonduer/candidates/mentions.py Outdated Show resolved Hide resolved
@lukehsiao lukehsiao merged commit e34f0be into HazyResearch:master Feb 14, 2020
@HiromuHota HiromuHota deleted the fix/316 branch June 6, 2020 00:12
@HiromuHota HiromuHota mentioned this pull request Sep 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@senwu senwu senwu approved these changes

@lukehsiao lukehsiao lukehsiao approved these changes

Assignees
No one assigned
Labels
clean-up Cleaning up the code or refactoring enhancement New feature or request
Projects
None yet
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.

No database dependency during inference/serving
3 participants
@HiromuHota @lukehsiao @senwu