Skip to content

Fix several errors found with ABECTO #526

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

Closed
wants to merge 1 commit into from
Closed

Conversation

jmkeil
Copy link
Contributor

@jmkeil jmkeil commented Jul 19, 2022

Hi. This fixes several errors found with the comparison using ABECTO (#525).

@jhodgesatmb
Copy link
Collaborator

jhodgesatmb commented Jul 20, 2022 via email

@steveraysteveray
Copy link
Collaborator

@jmkeil, thanks for the listing. I'm afraid we cannot just adopt the dimension vector changes you have in this PR, because our Units, QuantityKinds and DimensionVectors links involve more than just a given change for a QuantityKind. For example, for quantitykind:BodyMassIndex, you correctly point out that the length vector should be -2, not -1. However, in addition to that change, two Units point to quantitykind:BodyMassIndex (unit:KiloGM-PER-M and unit:KiloGM-PER-MilliM). They should no longer point to quantitykind:BodyMassIndex, so those triples should also be removed and replaced with references to quantitykind:MassPerArea (and others of like dimension as appropriate).

This also impacts our automatic generation of applicableUnits, as described here.

Finally, our QuantityKinds often are part of a skos:broader hierarchy where the broader and narrower entries all have the same dimensionality. That happens to not be the case with quantitykind:BodyMassIndex, but in general we need to move the quantity kind out of one skos:broader hierarchy and into another with the correct dimensionality.

We plan on correcting the errors you point out, but as I think you'll agree, this is more involved than one might think! We will proceed carefully.

@jmkeil jmkeil marked this pull request as draft July 22, 2022 07:06
@jmkeil
Copy link
Contributor Author

jmkeil commented Jul 22, 2022

@steveraysteveray thanks for reviewing the changes and the feedback. I wasn't aware of these hierarchy, but of course that needs to get adjusted, too. I converted the PR into a draft and you might use it as base for the further needed adjustments.

@steveraysteveray
Copy link
Collaborator

Thanks!
(and I didn't realize that Github supports draft PRs the way GitLab does!)

@jmkeil
Copy link
Contributor Author

jmkeil commented Jul 29, 2022

I just noticed, that I forgot to grant you edit privileges on this branch, but have made up for it now. Sorry.

Was it intended to close the PR?

@steveraysteveray
Copy link
Collaborator

Yes, I decided that rather than cherry-pick my way through your fixes, it was easier to close this PR, but work on a subset of the discrepancies in a new branch, which was merged on 2022-07-27. I started by addressing the dimension vectors because I felt that was structurally critical to get right. Each quantity kind noted in your spreadsheet takes research and cross-checking with other quantity kinds via skos:broader and with the units referring to it, so it's kind of slow. Of the 15 quantity kinds identified, I have addressed 10 of them in the latest merge.

I'm curious how your tool identifies the corresponding entities in the other ontologies, because, for example, what is known as "Magnetic Field" can be expressed in two different ways, related by Maxwell's equations. Our http://qudt.org/vocab/quantitykind/MagneticField describes the field in terms of B, and the OM ontology describes it in terms of H. They are not directly comparable. The OM version corresponds to what we call the AuxillaryMagneticField.

However, now that I have edit privileges on the branch, maybe I should go back and take advantage of your other changes as well, so I'm re-opening. Thanks.

@jmkeil
Copy link
Contributor Author

jmkeil commented Jul 29, 2022

I'm curious how your tool identifies the corresponding entities in the other ontologies, because, for example, what is known as "Magnetic Field" can be expressed in two different ways, related by Maxwell's equations.

The basic mapping is done based on String similarity of the labels (see c904f9b#diff-e48e569c0c233b06e9a61b27cf0f6ddedcd8142da4c91bf3d2f8e0debbbbc6b9R789) and is complemented by manual corrections in the pipeline configuration (see e.g. 6a06b8a#diff-e48e569c0c233b06e9a61b27cf0f6ddedcd8142da4c91bf3d2f8e0debbbbc6b9R748).

@steveraysteveray
Copy link
Collaborator

I have resolved the conflicts and incorporated your suggested changes that we agree with so far, and created a new branch and PR: "abecto errors". So I'm closing this one, having noted the remaining outstanding things to consider.

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