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

make models (plural) instead of make model #21

Closed
wants to merge 1 commit into from
Closed

Conversation

cneud
Copy link
Member

@cneud cneud commented Dec 4, 2020

Multiple models are indeed used plus this makes naming consistent with other Makefiles.

Multiple models are indeed used plus this makes naming consistent mit other Makefiles.
@mikegerber
Copy link
Member

I had a look on other Makefiles to check for consistency:

  • ocrd_calamari: Uses a different target name
  • ocrd_tesserocr: Does not have a model download in the Makefile
  • sbb_textline_detection: Does not have a model download in the Makefile
  • ocrd_cis: Does not have a model download in the Makefile

So I guess it's up to us how to name this. (There is resource management coming in OCR-D/core#559 but that only pertains to OCR-D.)

Copy link
Member

@mikegerber mikegerber left a comment

Choose a reason for hiding this comment

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

LGTM

@cneud
Copy link
Member Author

cneud commented Jan 25, 2021

I think I checked eynollah for a reference Makefile convention to use.

Moreover, the current version of sbb_binarization does indeed leverage multiple models - btw this it seems is also not properly exposed here.

Anyway I understand that meanwhile @vahidrezanezhad has trained a newer and uniform model so that future versions will only require a single model file...

@cneud cneud closed this Feb 4, 2021
@cneud cneud deleted the make-models branch April 20, 2022 15:01
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.

4 participants