Skip to content

make install-models: delegate to ocrd resmgr#234

Merged
kba merged 5 commits intomasterfrom
install-models-resmgr
Jan 30, 2021
Merged

make install-models: delegate to ocrd resmgr#234
kba merged 5 commits intomasterfrom
install-models-resmgr

Conversation

@kba
Copy link
Copy Markdown
Member

@kba kba commented Jan 18, 2021

Once OCR-D/core#559 is merged.

Copy link
Copy Markdown
Collaborator

@bertsky bertsky left a comment

Choose a reason for hiding this comment

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

I recommend a more radical change: Instead of unconditionally delegating install-models to install-models-ocropus, install-models-calamari etc, we should make these subtargets depend on whether the respective module is in the currently active OCRD_MODULES, and encapsulate each such rule near the other rules for the modules.

Thus, instead of …

ifneq ($(findstring ocrd_cis, $(OCRD_MODULES)),)
OCRD_EXECUTABLES += $(OCRD_CIS)
...

…we would have…

ifneq ($(findstring ocrd_cis, $(OCRD_MODULES)),)
install-models: install-models-cis
install-models-cis: $(BIN)/ocrd
	. $(ACTIVATE_VENV) && ocrd resmgr download ocrd-cis-ocropy-recognize '*'
OCRD_EXECUTABLES += $(OCRD_CIS)
...

Also, we still need to adapt Dockerfile:

ENV XDG_DATA_HOME /usr/local/share
VOLUME $XDG_DATA_HOME/ocrd-resources

(perhaps also HOME itself)

@kba
Copy link
Copy Markdown
Member Author

kba commented Jan 25, 2021

Having thought about it some more, I agree that install-models-* should depend on OCRD_MODULES to ensure that only the models are installed for those processors the user requested (by having it in OCRD_MODULES)

@kba
Copy link
Copy Markdown
Member Author

kba commented Jan 25, 2021

we should make these subtargets depend on whether the respective module is in the currently active OCRD_MODULES

fb4ee26

Also, we still need to adapt Dockerfile:

2fdce0a

Copy link
Copy Markdown
Collaborator

@bertsky bertsky left a comment

Choose a reason for hiding this comment

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

Splendid – thanks!

Perhaps we could offer more of these install-models-* rules in order to allow a quick installation without studying all processors and available models? (Currently, I would still have to ocrd resmgr download PROCESSOR '*' selectively for each processor I might want to use.)

Comment thread Makefile Outdated
kba and others added 2 commits January 25, 2021 13:45
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
@kba
Copy link
Copy Markdown
Member Author

kba commented Jan 25, 2021

Perhaps we could offer more of these install-models-*

384ee6d I added targets for all projects that we have in core's resource_list.yml.

You could always do the installation and then ocrd resmgr download '*' to download all models at once later on.

@kba kba mentioned this pull request Jan 29, 2021
@kba kba merged commit 8d38695 into master Jan 30, 2021
@kba kba deleted the install-models-resmgr branch April 25, 2021 10:49
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