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).
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.
Yes. Per @akanterhere, 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).
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.
@mseaton I have also run into a similar challenge related to a mix up of concepts that share the Fully Specified Names and the Short Names. I would think it wise indeed to have a solution that accommodates creating sets and coded questions with their members and answers as names in the CSVs because its easier to read for an implementor. We could have a custom method in Iniz to be able to have this strictness and if it is relevant elsewhere then we could probably modify the method in core to have a strict mode via a GP.
Or (as I thought you suggest on our internal Slack) Iniz could become configurable so that the concept names that are used as keys to identity concepts in CSVs:
Can either be any name (this is what happens now); or
Can only be FSNs (that would be the new configurable option).
I’d probably think that defaulting to FSNs makes the most sense and making the current behaviour the thing that can be enabled via configuration. That’s not backwards-compatible, but it seems more correct; even when we’re using human-readable identifiers (which should be the preference), it’s best if we’re using unambiguous ones.
Since this is primarily and pressingly intended for a Bahmni implementation, we suggest to stage the solution in Iniz for now, to avoid backporting as far as down to Core 2.1.x.
We’ve encountered an issue while setting up concept set members using concept names from a CSV file. Since the default locale consideration has been removed, we are now finding multiple concepts for the same concept name (FSN). For example, the concept name “hypertension” matches two CIEL concepts: CIEL:117399 (matched in default locale en) and CIEL:117401 (matched in the French locale).
Is there a specific reason why locale consideration was removed? Can it be reinstated?
Sure. Previously, if a concept name was specified, it would search for the name only in the default locale, as per the documentation .However, this default locale consideration has been removed. Pls check this change and related comment
Thank you for reporting this, @sanofersameera. When we implemented querying concepts by Fully Specified Names (FSNs) regardless of locale, we only envisioned such an edge use-case you’re sharing. We also had a workaround in mind for cases where two or more concepts shared the same FSN, using their UUIDs for reference in these edge cases. I also believe that such occurrences should be infrequent and not negate the benefits of the suggested workaround. Otherwise, it would raise concerns about the validity of the underlying concepts. An additional benefit to this is, one does not have to develop the concept configs in a specific locale of the target application using it.
This sounds concerning to me… it is important to disambiguate the terms with the language. We have traditionally shown the FSN (fully-specified name) and the synonym if a synonym is returned in the result. In this case, the Hypertension FSN in French is mapped to the Hypertensive disorder concept (in English). If we are going to shown names from a different locale, then they should also show the FSN in English.
English user:
Hypertension (en)
Hypertension (fr) → Hypertensive disorder (en)
French user:
Hypertension (fr)
Hypertension artérielle (fr)
But I guess the question is how you reference the concepts. You should not be using the name anyway, as those can actually change (minimally) but the underlying concept should not.
@sanofersameera how complicated would it be for you to start referencing concepts by UUIDs or same-as mappings instead of FSNs in your config(s)?
I think if we wanted to properly support referencing concepts by FSN we would have to provide an optional way to specify the locale applying to the referenced FSN.
Yes, the correct/best thing to here would be to start referencing by uuid or same-as mappings.
I’m assuming this is currently throwing an error because it finds two concepts with a FSN "hypertension”?
While annoying, I’d say throwing an error and asking for clarity is preferably to installing a different concept when installed on a system with a default locale in English vs a system with a default locale of French.