Skip to content

MS-988 Adding SimFace integration#1202

Merged
luhmirin-s merged 6 commits into
mainfrom
feature/MS-988-simface-integration
Jun 4, 2025
Merged

MS-988 Adding SimFace integration#1202
luhmirin-s merged 6 commits into
mainfrom
feature/MS-988-simface-integration

Conversation

@luhmirin-s
Copy link
Copy Markdown
Contributor

@luhmirin-s luhmirin-s commented May 28, 2025

JIRA ticket
Will be released in: 2025.2.0

Notable changes

  • Added SimFace SDK as a module.
  • Routed all calls to face configuration to the appropriate SDK configuration.
  • Added SDK parameter to face capture and matching flows similar to how it was already set up for fingerprint SDKs.
  • There is no need to add any additional flags since the SimFace configuration will only be provided to the projects in the dev-us deployment. For staging and prod projects, nothing changes.

Testing guidance

  • Once Vulcan has been updated, configure a project to use SimFace SDK for face modality capture. Run the usual biometric flows with the new SDK.

Additional work checklist

  • Effect on other features and security has been considered
  • Design document marked as "In development" (if applicable)
  • External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
  • Test cases in Testiny are up to date (or ticket created)
  • Other teams notified about the changes (if applicable)

@cla-bot cla-bot Bot added the ... label May 28, 2025
@luhmirin-s luhmirin-s force-pushed the feature/MS-988-simface-integration branch 2 times, most recently from 9684518 to b286120 Compare May 28, 2025 13:12
@luhmirin-s luhmirin-s requested a review from a team May 29, 2025 12:18
@luhmirin-s luhmirin-s marked this pull request as ready for review May 29, 2025 12:18
@luhmirin-s luhmirin-s requested review from BurningAXE, TristramN, alex-vt, alexandr-simprints, meladRaouf and ybourgery and removed request for a team May 29, 2025 12:18
@luhmirin-s luhmirin-s changed the title [WIP] MS-988 Adding SimFace integration MS-988 Adding SimFace integration May 29, 2025
@luhmirin-s luhmirin-s force-pushed the feature/MS-988-simface-integration branch 2 times, most recently from 6167762 to 76fc303 Compare June 3, 2025 10:18
Comment thread build-logic/build_properties.gradle.kts Outdated
* Dev version >= 2024.2.2 is required for float quality thresholds
* Dev version >= 2024.3.0 is required to receive configuration ID
* Dev version >= 2025.2.0 is required to support enrolment record updates
* Dev version >= 2025.2.0 is required to support enrolment record updates and biometric sdk age restrictions to receive SimFace configuration
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"biometric sdk age restrictions" - copy-paste?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤦🏻 yep

@luhmirin-s luhmirin-s force-pushed the feature/MS-988-simface-integration branch from 76fc303 to 7f07880 Compare June 3, 2025 13:53
candidate.faces.map { face ->
val baseScore = simFace.verificationScore(probeTemplate, face.template)
// TODO: remove the adjustment after we find out why the returned range is biased towards [0.5;1]
(baseScore - 0.5).coerceAtLeast(0.0).toFloat() * 200f
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not commenting on the "fix" itself :D but shouldn't this at least be inside the SDK?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It could be inside, but it is "simpler" to adjust it on the caller side as it does not require re-publishing of the AAR. Ideally, we find the reason for the bias and fix it on the model level (there is a plan on the biometrics team side). For now, this should be enough, tho.

Simber.i("Initialising matcher", tag = crashReportTag)
val bioSdk = resolveFaceBioSdk()
if (matchParams.faceSDK == null) {
send(MatcherState.Success(emptyList(), 0, ""))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we log a Simber.e() here (and in fingerprint's copy) as this would indicate a code error somewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will add Simber.w since there is no exception to log with .e().

val faces = simFace.detectFaceBlocking(bitmap)
val face = faces.getOrNull(0) ?: return@runBlocking null
// Skip the obviously bad images, but leave the rest to be determined by the caller
if (face.quality < 0.1) return@runBlocking null
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would be better if declared 0.1 in constants


dependencies {
implementation(project(":face:infra:base-bio-sdk"))
api(libs.simface)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you use implementation to avoid exposing the libs.simface outside this module ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, no - we need to provide the regular "SimFace" to the matcher's constructor.

licenseRepository.deleteCachedLicense(Vendor.RankOne)
_invalidLicense.send()
}
saveLicenseCheckEvent(licenseVendor, licenseStatus)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe in the future we can abstract all this logic in the license repo, so that it takes the SDK as param and returns whatever license is appropriate and the call here is the same regardless of the SDK.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I vote for removing the license module altogether. :D

Made a ticket for it: https://simprints.atlassian.net/browse/MS-1011

class SimFaceBioSdk @Inject constructor(
override val initializer: SimFaceInitializer,
override val detector: SimFaceDetector,
private val simFace: SimFace,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a little weird to have SimFace* and just SimFace. However, the latter comes from the lib so don't have a concrete suggestion at the moment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is only due to the matchers being created dynamically for the probe list.

Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE left a comment

Choose a reason for hiding this comment

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

LGTM

@luhmirin-s luhmirin-s force-pushed the feature/MS-988-simface-integration branch from 7f07880 to be324d3 Compare June 3, 2025 15:17
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 3, 2025

@luhmirin-s luhmirin-s merged commit 2683264 into main Jun 4, 2025
12 checks passed
@luhmirin-s luhmirin-s deleted the feature/MS-988-simface-integration branch June 4, 2025 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants