-
Notifications
You must be signed in to change notification settings - Fork 50
new assesment #137
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
base: develop
Are you sure you want to change the base?
new assesment #137
Conversation
1eca103 to
1fce626
Compare
b20d223 to
d4e6312
Compare
| image: | ||
| target_height: "" | ||
| target_width: "" | ||
| granular: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look backward compatible. Is it? We do not want to break the existing granular assessment. Or do you propose that we replace 'granular assessment' (our current default) with 'agentic assessment'?
| - Provide tight, accurate bounding boxes around the actual text | ||
| </assessment-guidelines> | ||
| <spatial-localization-guidelines> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, won't removing this break the current assessment implementation?
| The module supports both: | ||
| 1. Original approach: Single inference for all attributes in a section | ||
| 2. Granular approach: Multiple focused inferences with caching and parallelization | ||
| All assessment now uses the granular approach with Strands agents for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this answers my question above.. you are replacing granular assessment with agentic assessment.. High stakes - we have customers (like Deluxe) relying on this.. if the new method consistently works better and doesn't affect cost, then i agree this is cleaner, more maintainable, approach.. But you'll need to do tests to prove that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify the changes to the UI? I was not expecting any UI changes, and hard to assess the nature/purpose from code read.
Mostly, can you confirm that changes are backward compatible with processed docs using the previous method, and especially with pattern-1 BDA output.. Or put differently, can you confirm that the JSON structure for explainability_info in the results.json is unchanged?
|
Too many code changes for me to make sense of, but I did ask a few questions.. can you replay inline. Tx! Also, as we discussed today, Nova 2 is specialized in geometry.. See https://docs.aws.amazon.com/nova/latest/nova2-userguide/prompting-multimodal.html#:~:text=Detect%20objects%20and%20their%20positions%20in%20images |
6a17108 to
12fb11e
Compare
12fb11e to
acd8967
Compare
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.