Initializer, Matching Concepts by Name and the OpenMRS getConceptByName method

Hello all…

We are currently trialing moving some of the concept configuration for one of our implementations out of Metadata Sharing and into Iniz, and ran into one potential problem/risk:

When referencing a concept (for example, when setting up concept sets or concept answers), Iniz supports referencing that concept by uuid, mapping, or name, and referencing by name is a common choice because it leads to the most “readable” CSV files.

Behind-the-scenes, Iniz delegates to the ConceptService.getConceptByName method to look up concepts based on name. We’ve noticed some perhaps unexpected behavior with this method when there are two concepts that share the same name. For example, if you have a concept with a Fully Specified Name in a locale, and that same name is also a Synonym for different concept, there’s no guarantee which of the two concepts will be returned. With Iniz, this could result with a wrong concept being associated with an concept set or an answer.

Some Thoughts:

  • We should consider changing the getConceptByName so that if there are multiple concepts with the same name, it picks which one to return based on a standard set of rules (assumedly preferring the concept with the FSN = name)

  • We should consider changing Iniz so that it can set to a “strict” mode, in which case it will only match on Fully Specified Name, to avoid choosing an unintended name

(We are not blocked on this, and have worked around the problem by using concept mappings for the few cases in our system where this is a problem, but wanted to share and throw this out there for others to consider).

fyi @mseaton @burke @mksd @mksrom @dkayiwa @ibacher

Take care, Mark

1 Like

Thanks @mogoodrich for raising these.

I agree with this, but also think we might consider going further and deprecating this method altogether, as there is no way to know whether the concept returned is the concept intended across all use cases. This is actually how the ConceptService.getConceptByMapping method works - if there are > 1 non-retired concepts that could match the given mapping, the method throws an Exception. Still, even here there may be scenarios where the retired concept is actually the one you want to refer to…

Agreed, and we should encourage implementations to adopt a “strict” mode in their configuration settings.

1 Like

Thanks @mseaton … thinking about it, I tend to agree with deprecating the getConceptByName altogether in favor of a clearer method.

1 Like

This breaks our concept validation rules. If a name can be used for multiple concepts, it is not fully specified:

  1. Fully specified names must be unique across all names (except short names and index terms) in a locale

Is this true @burke ? Isn’t @mogoodrich talking about a name that exists as an FSN on one concept and as an index term (synonym) on another?

1 Like

Yes. Per @akanter here, if a name is fully specified, then it shouldn’t refer to something else.

@ball pointed out cases where even CIEL has challenges with this rule, when an FSN is used for general terms (like Absent), that may be useful as synonyms elsewhere. But I think we have fixed all of those cases in CIEL.

I assume Andy’s recommendation for when these conflicts arise would be to fix the FSN to be more fully specified – e.g., change “Grey” to “Grey color” and make “Grey” a synonym & preferred (so concept still shows as “Grey” but has a fully specified name that is less ambiguous).

1 Like

Thanks @burke . I re-read that thread, which is useful. Not sure whether to comment further on this thread or on that one, but probably makes sense to do it on that one, and then we can try to pull back any further information here if needed. That said, this discussion is coming up because the validation you describe above is not enforced in OpenMRS. So dictionaries have situations where the same name is used as a fully specified name for one concept, and a synonym for another, and this caused issues in matching that concept in Iniz, which is the reason @mogoodrich raised this post.