Closed juhoinkinen closed 1 year ago
Base: 99.55% // Head: 89.44% // Decreases project coverage by -10.11%
:warning:
Coverage data is based on head (
ec16a21
) compared to base (ca4d61c
). Patch coverage: 97.94% of modified lines in pull request are covered.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Some open thoughts & questions:
DocumentList
object made the implementation for ìndex
command a bit easier that using plain list of texts. But actually, instead of using DocumentList
class there could be a DocumentBatch
class just for this use, but would that give any benefits. batch-suggest
would be a better name for the method than suggest-batch
.I reverted the commits about REST API (i.e. adding the /v1/suggest-batch
method) and cherry-picked them to a new branch issue579-batch-suggest-operation-rest-api. This PR was already quite large, and might grow further if batching in other CLI commands is implemented.
I tried to utilize the new suggest_batch
function of the project module for the eval
CLI command, but it does not seem very straightforward.
First, the eval command uses the imap_unordered, which expects an iterable as the second argument), but suggest_batch
operates on a corpus object (that has documents
iterable only as its property). Also, I'm not sure if imap_unordered
could in any way feed multiple documents to the suggest_batch
function: it has the chunksize
parameter, but afaik even when setting that to a non-default value (!=1) it does not send multiple elements from the iterable to the function in one pass.
I added BatchingDocumentCorpus
to help getting batches of documents, but I wonder could the doc_batches(batch_size)
method be added already to the DocumentCorpus
base class, as batching of documents is used quite many times. (TODO: add testing of the batching method.)
(Black v23.1.0 was just released, and it introduced some changes to the style, which raise complaints for the current Annif code. :( )
I wonder could the doc_batches(batch_size) method be added already to the DocumentCorpus base class, as batching of documents is used quite many times.
Yes, this sounds like a great idea! I think it would simplify the code and enable batching in many places.
Oh by the way, I tested the annif eval
command (yso-mllm-fi project, evaluated on kirjastonhoitaja test set) with the -j 4
option, before and after this PR. The evaluation results were the same. It took a few seconds longer with the code in this PR, probably because the parallel processing is done on larger batches and the final batches cannot be distributed between CPUs. But I think that's not too bad, and we should see performance gains in the future as we implement support for batching in individual backends.
Now some debug-level logs from project module are removed: Before:
annif suggest tfidf-fi <kissa.txt -v DEBUG
debug: creating app with configuration annif.default_config.Config
debug: Reading configuration file projects.cfg in CFG format
debug: loading subjects from data/vocabs/yso/subjects.csv
debug: Suggesting subjects for text "kissa
debug: ..." (len=6)
debug: Backend tfidf: loading vectorizer from data/projects/tfidf-fi/vectorizer
debug: Backend tfidf: loading similarity index from data/projects/tfidf-fi/tfidf-index
debug: Backend tfidf: Suggesting subjects for text "kissa
debug: ..." (len=6)
debug: Got 100 hits from backend tfidf
debug: 100 hits from backend
<http://www.yso.fi/onto/yso/p19378> kissa 0.9530429244041443
<http://www.yso.fi/onto/yso/p864> kissaeläimet 0.5541983842849731
Now:
annif suggest tfidf-fi <kissa.txt -v DEBUG
debug: creating app with configuration annif.default_config.Config
debug: Reading configuration file projects.cfg in CFG format
debug: loading subjects from data/vocabs/yso/subjects.csv
debug: Backend tfidf: loading vectorizer from data/projects/tfidf-fi/vectorizer
debug: Backend tfidf: loading similarity index from data/projects/tfidf-fi/tfidf-index
debug: Backend tfidf: Suggesting subjects for text "kissa
debug: ..." (len=6)
<http://www.yso.fi/onto/yso/p19378> kissa 0.9530429244041443
<http://www.yso.fi/onto/yso/p864> kissaeläimet 0.5541983842849731
And in the case giving multiple files to annif suggest
, the debug log has some duplication:
annif suggest tfidf-fi *.txt -v DEBUG
debug: creating app with configuration annif.default_config.Config
debug: Reading configuration file projects.cfg in CFG format
debug: loading subjects from data/vocabs/yso/subjects.csv
debug: Backend tfidf: loading vectorizer from data/projects/tfidf-fi/vectorizer
debug: Backend tfidf: loading similarity index from data/projects/tfidf-fi/tfidf-index
debug: Backend tfidf: Suggesting subjects for text "kissa
debug: cat
debug: dog
debug: viiki..." (len=24)
debug: Backend tfidf: Suggesting subjects for text "kissa
debug: ..." (len=6)
debug: Backend tfidf: Suggesting subjects for text "koira
debug: ..." (len=6)
debug: Backend tfidf: Suggesting subjects for text "laiva
debug: ..." (len=6)
debug: Backend tfidf: Suggesting subjects for text "viikinki
debug: ..." (len=9)
Now some debug-level logs from project module are removed
Right, because the debug information was printed only in the single text suggest
methods. I don't think this is a big loss.
And in the case giving multiple files to annif suggest, the debug log has some duplication:
I don't see any exact duplicates - the messages are related to different input files, right?
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells
No Coverage information
0.0% Duplication
Adds support for passing multiple documents in a batch from
suggest
,index
,eval
andoptimize
CLI commands to backends (like discussed in issue #579).Makes
annif suggest
accept path(s) to file(s) to be indexed, in addition to stdin:The output for each file path input begins with line
Suggestions for <file/path>
:The documents are passed as batches, i.e. lists of texts (lists are generated by a generator also in the case of
suggest
command with files) to thebackend.py
module, which defines the default_suggest_batch
method that uses the regular, single-text_suggest
method. This allows the actual backends to define their own_suggest_batch
methods that can operate on the document batch.The implementation of document batching for the
hyperopt
CLI command is left from this PR, as the hyperopt functionality is implemented in individual backends.Note that there is no support the actual batch-processing in any backends yet.