Rewrite llm query operator #1216
Conversation
| if self.include_image and len(result.messages) > 0: | ||
| from sycamore.utils.pdf_utils import get_element_image | ||
|
|
||
| result.messages[-1].images = [get_element_image(elt, doc)] |
There was a problem hiding this comment.
might also want to add the image of prev here?
| skips = [] | ||
| counter = 0 | ||
| for e, _ in elt_doc_pairs: | ||
| if self._filter(e) and (not self._number_of_elements or counter < self._number_of_elements): | ||
| counter += 1 | ||
| else: | ||
| skips.append(e) |
There was a problem hiding this comment.
skips should be an array of bools (with length = num of elements) if the rest of its usage remains the same
| skips = [] | ||
| counter = 0 | ||
| for e, _ in elt_doc_pairs: | ||
| if self._filter(e) and (not self._number_of_elements or counter < self._number_of_elements): |
There was a problem hiding this comment.
Why are we filtering twice when I want to run Table Merger? Once here for table and then again in render_elements.
There was a problem hiding this comment.
For table merger, I think mostly, the prompt side does not have the filter or skip information that's collected in the llm_map_elements.
| self._user_templates: Union[None, list[Template]] = None | ||
|
|
||
| def render_element(self, elt: Element, doc: Document) -> RenderedPrompt: | ||
| filtered = [e for e in doc.elements if e.type == "table"] |
There was a problem hiding this comment.
Am I understanding this wrong or for every document we have O(len(doc.elements)^2)?
There was a problem hiding this comment.
yes, seems N square, any suggestion to avoid this?
There was a problem hiding this comment.
@HenryL27 no wiggle room in render_elements for this?
There was a problem hiding this comment.
I think the alternative is to do a single pass beforehand and write down a pointer on each element to prev.
then in render elements you're O(1) right?
There was a problem hiding this comment.
You might be able to avoid needing this particular class entirely since you can do that lookup in Jinja - something like
"""
{%- set prev = doc.elements[elt.properties["_prev_table"]] if "_prev_table" in elt.properties else None -%}
"""tho I guess if you want to include the image of prev you need to know prev in the render fn
bohou-aryn
force-pushed
the
llm_query
branch
2 times, most recently
from
d6ff7c5 to
969d929
Compare
This is part of the series of tasks for converting llm operator to use Henry's new LLMMap or LLMElementMap.
This is part of the series of tasks for converting llm operator to use Henry's new LLMMap or LLMElementMap.