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

Add maxSnippetSize parameter #128

Closed
margaretha opened this issue Mar 20, 2024 · 4 comments
Closed

Add maxSnippetSize parameter #128

margaretha opened this issue Mar 20, 2024 · 4 comments
Assignees

Comments

@margaretha
Copy link
Contributor

margaretha commented Mar 20, 2024

Please add a new parameter to allow Kustvakt to change the snippet size beyond limit. It is necessary to support larger match context for a group of users see (KorAP/Kustvakt#745)

The parameter should be exclusive for Kustvakt and not adjustable by users.

@margaretha margaretha changed the title Add maximumSnippetSize parameter Add maxSnippetSize parameter Mar 20, 2024
@Akron
Copy link
Member

Akron commented Mar 21, 2024

I checked and there are actually two limits: one is character based, one is token based. The match has a token based limit, which makes sense for annotation data retrieval. so we may want to have maxMatchTokenSize. And we have context limits, which are character based, which may make sense as well ... So - for this possibly maxContextCharSize?

@margaretha
Copy link
Contributor Author

Thanks for checking, Nils! Does maxMatchTokenSize not include the contexts? Just the matches itself?

whereas maxContextCharSize include match and context alltogether?

@Akron
Copy link
Member

Akron commented Mar 26, 2024

No - maxMatchTokenSize doesn't include the context and maxContextCharSize is a maximum value for left and right context. There is no maximum snippet size, as we allow to cut matches and still allow to view the context. I can't remember the concrete reason, it may have been just simpler to implement. But it also has some advantages.

Krawfish will have some difficulties with character sizes in contexts, but I think we can keep this feature.

So - do you want to have the context adjustable only?

@margaretha
Copy link
Contributor Author

Thanks for the clarification! We have discussed this over Slack and agree to allow adjustment for both variables.

idsgerrit pushed a commit that referenced this issue May 7, 2024
Uploaded patch set 5.

Patch-set: 5
Subject: Make match size configurable (address #128)
Commit: 7990990
Tag: autogenerated:gerrit:newPatchSet
Groups: 7990990
idsgerrit pushed a commit that referenced this issue May 7, 2024
Change-Id: I8c542a7fb5406b9bb7d08b909114a99348ff84e9
idsgerrit pushed a commit that referenced this issue May 14, 2024
Change-Id: I8c542a7fb5406b9bb7d08b909114a99348ff84e9
idsgerrit pushed a commit that referenced this issue May 14, 2024
Uploaded patch set 6.

Patch-set: 6
Subject: Make match size configurable (address #128)
Commit: da16975
Tag: autogenerated:gerrit:newPatchSet
Groups: da16975
idsgerrit pushed a commit that referenced this issue May 15, 2024
Change-Id: I81eacce8fb0991ed2c2470950f36cfbc02b9ea7f
idsgerrit pushed a commit that referenced this issue May 15, 2024
Uploaded patch set 5.

Patch-set: 5
Subject: Make search context configurable to set max token length (#128).
Commit: afd2035
Tag: autogenerated:gerrit:newPatchSet
Groups: da16975
idsgerrit pushed a commit that referenced this issue May 16, 2024
Uploaded patch set 7.

Outdated Votes:
* Code-Review+1 (copy condition: "changekind:NO_CHANGE OR is:MIN")


Patch-set: 7
Subject: Make match size configurable (address #128)
Commit: 524e318
Tag: autogenerated:gerrit:newPatchSet
Groups: 524e318
Attention: {"person_ident":"Gerrit User 19 \u003c19@626241b7-b7d0-4197-b843-1ee23887649c\u003e","operation":"ADD","reason":"Vote got outdated and was removed: Code-Review+1"}
idsgerrit pushed a commit that referenced this issue May 16, 2024
Change-Id: I8c542a7fb5406b9bb7d08b909114a99348ff84e9
idsgerrit pushed a commit that referenced this issue May 17, 2024
Change-Id: Ieef96dd68adf4e3ce00f59fc21face545c2ce897
idsgerrit pushed a commit that referenced this issue May 17, 2024
Uploaded patch set 1.

Patch-set: 1
Change-id: Ieef96dd68adf4e3ce00f59fc21face545c2ce897
Subject: Make match size configurable (address #128)
Branch: refs/heads/master
Status: new
Topic: 
Commit: bcb8906
Tag: autogenerated:gerrit:newPatchSet
Groups: bcb8906
Private: false
Work-in-progress: false
idsgerrit pushed a commit that referenced this issue May 17, 2024
Change-Id: Ieef96dd68adf4e3ce00f59fc21face545c2ce897
idsgerrit pushed a commit that referenced this issue May 17, 2024
Uploaded patch set 2.

Outdated Votes:
* Code-Review+1 (copy condition: "changekind:NO_CHANGE OR is:MIN")


Patch-set: 2
Subject: Make match and context size configurable (address #128)
Commit: 9b8ec14
Tag: autogenerated:gerrit:newPatchSet
Groups: 9b8ec14
Attention: {"person_ident":"Gerrit User 21 \u003c21@626241b7-b7d0-4197-b843-1ee23887649c\u003e","operation":"ADD","reason":"Vote got outdated and was removed: Code-Review+1"}
idsgerrit pushed a commit that referenced this issue May 21, 2024
Uploaded patch set 3.

Outdated Votes:
* Code-Review+1 (copy condition: "changekind:NO_CHANGE OR is:MIN")


Patch-set: 3
Subject: Make match and context size configurable (address #128)
Commit: 464ae45
Tag: autogenerated:gerrit:newPatchSet
Groups: 464ae45
Attention: {"person_ident":"Gerrit User 19 \u003c19@626241b7-b7d0-4197-b843-1ee23887649c\u003e","operation":"ADD","reason":"Vote got outdated and was removed: Code-Review+1"}
idsgerrit pushed a commit that referenced this issue May 21, 2024
Change-Id: Ieef96dd68adf4e3ce00f59fc21face545c2ce897
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

No branches or pull requests

2 participants