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

update 2020-08-21 #175

Merged
merged 3 commits into from
Aug 21, 2020
Merged

update 2020-08-21 #175

merged 3 commits into from
Aug 21, 2020

Conversation

kba
Copy link
Member

@kba kba commented Aug 21, 2020

This is a significant update of core and most processors to adhere to OCR-D/spec#164 and use the helpers introduced in OCR-D/core#530.

Here's how I kept track of the changes, hopefully I haven't overlooked any processor:

project PR Release
cor-asv-ann ASVLeipzig/cor-asv-ann#4 -
cor-asv-fst ASVLeipzig/cor-asv-fst#5 -
dinglehoppper qurator-spk/dinglehopper#23 master
ocrd_anybaseocr OCR-D/ocrd_anybaseocr#64 1.0.0
ocrd_calamari https://github.com/OCR-D/ocrd_calamari/pull/42/files 0.1.0
ocrd_cis cisocrgroup/ocrd_cis#58 TODO
ocrd_fileformat - -
ocrd_im6convert - -
ocrd_keraslm OCR-D/ocrd_keraslm#17 0.4.0
ocrd_olena - 1.2.1
ocrd_kraken - -
ocrd_ocropy - -
ocrd_pagetopdf - -
ocrd_pc_segmentation ocr-d-modul-2-segmentierung/ocrd-pixelclassifier-segmentation#11 master
ocrd_tesserocr OCR-D/ocrd_tesserocr#133 0.9.1
ocrd_typegroups_classifier OCR-D/ocrd_typegroups_classifier#2 0.1.0
ocrd_wrap bertsky/ocrd_wrap@6f7f785 TODO
sbb_textline_detector - -
ocrd_segment OCR-D/ocrd_segment#41 0.1.0

@kba
Copy link
Member Author

kba commented Aug 21, 2020

CI initially failed because core 2.13.2 wasn't available from PyPI. This needs a fix along the lines of #170 (comment)

@kba
Copy link
Member Author

kba commented Aug 21, 2020

Failed for a second time because a typo in ocrd_typegroups_classifier setup.py, fixed now. Also added -j to the make docker-maximum call for the PR building branch of the CI script.

@kba
Copy link
Member Author

kba commented Aug 21, 2020

AFAICS the -j flag is a no-op here because the container runs with just 2 cores.

@stweil
Copy link
Collaborator

stweil commented Aug 21, 2020

Using make -j without additional flags which limit the number of processes is expected to fail with out-of-memory.

@kba
Copy link
Member Author

kba commented Aug 21, 2020

Using make -j without additional flags which limit the number of processes is expected to fail with out-of-memory.

No, at least not in the Circle CI Docker container: https://app.circleci.com/pipelines/github/OCR-D/ocrd_all/335/workflows/84e86752-fb96-4927-a232-bcdad45d6e34/jobs/336

@kba kba merged commit 19edcd1 into master Aug 21, 2020
@kba kba deleted the update-2020-08-21 branch August 21, 2020 15:51
@stweil
Copy link
Collaborator

stweil commented Aug 21, 2020

CI runs make -j docker-maximum now, but that starts only a single process docker build. So in this special case -j is really a no-op on any host, no matter how many CPU cores are available. You'd get the promised OOM crash if you used make -j in Dockerfile.

@stweil
Copy link
Collaborator

stweil commented Aug 21, 2020

I suggest to remove the related commit. Adding -j without any effect is just confusing.

@kba
Copy link
Member Author

kba commented Aug 21, 2020

I'll prepare a PR with a revert but let's keep it out of master until https://app.circleci.com/pipelines/github/OCR-D/ocrd_all/336/workflows/61f0e209-2785-45fb-9037-0d9610fd3431 finished.

@kba
Copy link
Member Author

kba commented Aug 21, 2020

#176

@bertsky
Copy link
Collaborator

bertsky commented Aug 21, 2020

CI runs make -j docker-maximum now, but that starts only a single process docker build

We could be even more bold and use make -j all in Dockerfile in general!

@stweil
Copy link
Collaborator

stweil commented Aug 21, 2020

I never tried that in Dockerfile, but expect that it will crash with OOM. make all -j2 would be reasonable as there are obviously two cores available.

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