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

QA Spec #225

Merged
merged 57 commits into from
Mar 1, 2023
Merged

QA Spec #225

merged 57 commits into from
Mar 1, 2023

Conversation

kba
Copy link
Member

@kba kba commented Sep 13, 2022

This pull request offers our first draft for the QA Specs. It consists of two main parts:

@kba kba marked this pull request as ready for review September 26, 2022 07:39
ocrd_eval.schema.json Outdated Show resolved Hide resolved
ocrd_eval.schema.json Outdated Show resolved Hide resolved
ocrd_eval.schema.json Outdated Show resolved Hide resolved
ocrd_eval.schema.json Outdated Show resolved Hide resolved
ocrd_eval.schema.yml Outdated Show resolved Hide resolved
ocrd_eval.schema.yml Outdated Show resolved Hide resolved
ocrd_tool.schema.json Outdated Show resolved Hide resolved
ocrd_tool.schema.json Outdated Show resolved Hide resolved
ocrd_tool.schema.json Outdated Show resolved Hide resolved
ocrd_tool.schema.json Outdated Show resolved Hide resolved
@mweidling
Copy link
Contributor

LGTM!

ocrd_eval.schema.json Outdated Show resolved Hide resolved
ocrd_eval.schema.yml Outdated Show resolved Hide resolved
kba and others added 4 commits September 27, 2022 11:59
Co-authored-by: mweidling <13831557+mweidling@users.noreply.github.com>
Co-authored-by: mweidling <13831557+mweidling@users.noreply.github.com>
Copy link
Member Author

@kba kba left a comment

Choose a reason for hiding this comment

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

I've merged all your proposals AFAICT.

For the future please modify only the YAML files, the JSON files are generated from them. I can also generate the JSON in a non-pretty-printed format to reduce confusion.

ocrd_eval.schema.yml Outdated Show resolved Hide resolved
ocrd_eval.schema.yml Outdated Show resolved Hide resolved
@mweidling
Copy link
Contributor

I've merged all your proposals AFAICT.

For the future please modify only the YAML files, the JSON files are generated from them. I can also generate the JSON in a non-pretty-printed format to reduce confusion.

Yeah, I noticed that too – too late. 🤪 Will do in the future!

@M3ssman
Copy link

M3ssman commented Oct 7, 2022

Just to make it as clear as possible: a character regarding these definitions is a glyph? Something printable visual, a graphical representation of a character? Saying so, any special whitespace codepoint (spatium, tab, zero-width spatium, invisible times, ... ) is not a character regarding OCR-D QA?

IMHO this is quite reasonable.

This doesn't apply to word-based metrics. But since usually structured GT shall be the backbone for evaluation, word boundaries or words at all are present already in the data if it is present at least on word level.

Since this implies concerning character-based textual evaluation to strip off any spaces forehand, it should be cleared on which level (line with spaces or finer) both GT and related candidate data are available.

If GT is for whatever reasons only on line-level present, I assume that these spaces are normalized too or even inserted by some legacy tooling meaning there's no reason either to keep these code points either.

ocrd_eval.sample.yml Outdated Show resolved Hide resolved
@paulpestov
Copy link

Due to ongoing changes in the ocrd_eval schema I think we should omit those changes for this PR to separate the different issues that this PR is trying to solve: defining a first draft of metrics definitions and creating a JSON schema for the Quiver API. Since the API is still at a very early stage I'm not quite sure if it generates that much value for us if we create a spec right now.

@mweidling
Copy link
Contributor

Due to ongoing changes in the ocrd_eval schema I think we should omit those changes for this PR to separate the different issues that this PR is trying to solve: defining a first draft of metrics definitions and creating a JSON schema for the Quiver API. Since the API is still at a very early stage I'm not quite sure if it generates that much value for us if we create a spec right now.

I second that. Since the requirements for the UI are not completely clear yet, we should move the JSON schema for the data to be delivered by the back end to a separate branch.

@kba
Copy link
Member Author

kba commented Dec 19, 2022

I second that. Since the requirements for the UI are not completely clear yet, we should move the JSON schema for the data to be delivered by the back end to a separate branch.

Fine with me.

kba added a commit that referenced this pull request Dec 19, 2022
@kba kba mentioned this pull request Dec 19, 2022
@kba
Copy link
Member Author

kba commented Dec 19, 2022

Schema changes now in #236

Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Show resolved Hide resolved
mweidling and others added 6 commits February 14, 2023 10:22
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Show resolved Hide resolved
ocrd_eval.md Show resolved Hide resolved
mweidling and others added 7 commits February 15, 2023 08:11
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
Copy link
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.

devil in the details...

ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
Copy link
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 can live with the PR as it is now.

If you want to elaborate in the definition BoW metrics, fine. If BWE should be replaced with BoW-Precision and BoW-Recall, also fine – as long as you give a concrete definition (ideally also mentioning which existing implementations provide which precise measure).

More references you might want to include:

ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
ocrd_eval.md Outdated Show resolved Hide resolved
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
@kba kba merged commit a3505d7 into master Mar 1, 2023
@kba kba deleted the qa-spec branch March 1, 2023 12:04
@kba
Copy link
Member Author

kba commented Mar 1, 2023

Merged and wil release it later. There are still open questions and "postponed" metrics but it is an excellent first version we can and will build upon.

@kba
Copy link
Member Author

kba commented Mar 1, 2023

If I missed an unresolved discussion or some aspect that should be tracked in a dedicated issue, please let me know and/or open an issue.

@bertsky
Copy link
Collaborator

bertsky commented Mar 1, 2023

The only open discussion is the one about BoW metrics. It's still somewhat valuable because it shows which implementations use which definitions. (Or should we add this to our Evaluation Wiki page?)

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.

5 participants