Skip to content

Conversation

@rgupta2508
Copy link

@rgupta2508 rgupta2508 commented Feb 5, 2025

Reopen this PR #1508
Creating map of supported entity and recognizers and filtering out based on entity name using kay .

Code refactor.

Improve complexity of filter out unwanted recognizers from O(n*m )to O(n)

In current code for loop is running inside for loop that makes complexity to O(n*m). after this change there is only one loop by using one extra variable.
where
m = total number of all_possible_recognizers
n= total supported entities which we want to include .

Change Description

Describe your changes

Issue reference

This PR fixes issue #XX

Checklist

  • I have reviewed the contribution guidelines
  • I have signed the CLA (if required)
  • My code includes unit tests
  • All unit tests and lint checks pass locally
  • My PR contains documentation updates / additions if required

@rgupta2508
Copy link
Author

@microsoft-github-policy-service agree

@omri374 omri374 requested a review from Copilot February 23, 2025 13:30
@omri374
Copy link
Contributor

omri374 commented Feb 23, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

1 similar comment
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@omri374
Copy link
Contributor

omri374 commented Mar 10, 2025

@rgupta2508 after some thinking, we've decided to close this PR. We appreciate the change, but in the tradeoff between computational efficiency and code complexity, we've decided that it's a bit risky. The number of combinations here (entities, languages, recognizers) is not high therefore we don't expect a significant performance boost here. We would be happy to be corrected otherwise if you did any analysis on this.

@omri374 omri374 closed this Mar 10, 2025
@rgupta2508
Copy link
Author

rgupta2508 commented Mar 24, 2025

Hi @omri374,
Thanks for review.
We s a walmart team, having lot of recognizers (multiple entity in one recognizers) in different language.
We are using this for each llm calls( more then 20 million call a day). so each request having different recognizers/entity(based on the usecase) in different language .
We observed significant improvement wrt time to take run for these data. and our llm prompt response time also improved .

@omri374
Copy link
Contributor

omri374 commented Mar 25, 2025

Hi @rgupta2508

Thanks for this input! We will continue reviewing this and get back to you.

@omri374 omri374 reopened this Mar 25, 2025
@rgupta2508
Copy link
Author

Hi @omri374 Any updates regarding this PR?

@omri374
Copy link
Contributor

omri374 commented Apr 15, 2025

hi @rgupta2508, apologies for the delay

@omri374
Copy link
Contributor

omri374 commented Apr 15, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@omri374 omri374 requested a review from Copilot April 15, 2025 06:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.


return list(to_return)

def add_recognizer_map(self, all_entity_recognizers, supported_entity, rec):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please make this private + add type hints?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

2 participants