Add area and centroid calculation #1173
Conversation
jeffbl
left a comment
jeffbl
left a comment
There was a problem hiding this comment.
Approving, but please consider comments before merging. Some are "future us" problems like the timeout, but I wanted to have that issue refer to this one, since it probably won't be obvious that timeout is overridden directly in Dockerfile when we eventually implement #1077.
| HEALTHCHECK --interval=60s --timeout=10s --start-period=120s --retries=5 CMD curl -f http://localhost:5000/health || exit 1 | ||
|
|
||
| CMD [ "gunicorn", "object-detection-llm:app", "-b", "0.0.0.0:5000", "--capture-output", "--log-level=debug" ] No newline at end of file | ||
| CMD [ "gunicorn", "object-detection-llm:app", "-b", "0.0.0.0:5000", "--capture-output", "--log-level=debug", "--timeout", "75"] No newline at end of file |
There was a problem hiding this comment.
Timeout adjustments should really be done in docker-compose, but that isn't implemented until #1077 is complete.
There was a problem hiding this comment.
Is there anything I can do about it now?
There was a problem hiding this comment.
I can't think of anything since #1077 is not implemented. But at least these comments should show up there now. :)
| restart: "no" | ||
| environment: | ||
| - CONF_THRESHOLD=0.9 | ||
| - CONF_THRESHOLD=0.8 |
There was a problem hiding this comment.
Could argue this should be settable in docker-compose, so that it can be tuned by people who have different requirements for number of items vs. accuracy, or who change the model used, and the confidence scale shifts.
As discussed in slack earlier, filtering should also really be a handler issue in the long-run. So confidence threshold should be tuned to "too much" rather than "too little" and individual handlers can decide what tradeoffs they want to make.
There was a problem hiding this comment.
This is settable in docker-compose, or am I missing something?
Do you want me to change the threshold to 0 and let handers decide what they want to do?
There was a problem hiding this comment.
No, go with a threshold you're comfortable with, since we don't have anyone to implement handler changes at this point. But this needs to be resolved in the future. And, great it is settable in docker-compose. Didn't remember that!
utils/llm/prompts.py
Outdated
| 3. Use simple and common object labels (e.g., "car", "person", "tree"). | ||
| 4. Include only objects that are clearly visible and identifiable. | ||
| 5. Focus on the major and important objects in the image. | ||
| 6. Several objects can have the same confidence score. |
There was a problem hiding this comment.
Reading this as a human, I get what you mean, but literally, "several" can be interpreted as 3, or more vaguely, a small number less than 5. I don't know if this has potential to confuse LLM, but "Multiple" is more accurate since it does not imply a low number.
There was a problem hiding this comment.
Makes sense, thank you. Fixed.
| for idx, obj in enumerate(filtered): | ||
| obj['ID'] = idx | ||
|
|
||
| x1, y1, x2, y2 = obj["dimensions"] |
There was a problem hiding this comment.
I wouldn't expect to find this in a function "filter_objects" since it has nothing to do with filtering. make function name more generic?
|
Before merging, assigning to @jeffbl to make sure I understood all comments correctly and implemented all needed fixes. |
object-detection-llmpreprocessor.object-detection-llmtimeout to 75 seconds to allow for processing of complex graphics.Required Information
Coding/Commit Requirements
New Component Checklist (mandatory for new microservices)
docker-compose.ymlandbuild.yml..github/workflows.README.mdfile that describes what the component does and what it depends on (other microservices, ML models, etc.).OR