Chore/opti eval #38

Ben-Epstein · 2023-09-18T01:05:11Z

No description provided.

Update eval_with_hnsw.py

…hore/opti-eval

Ben-Epstein · 2023-09-18T11:17:34Z

dalm/eval/eval_retriever_only.py

@@ -104,9 +104,11 @@ def main() -> None:
    args = parse_args()
    SELECTED_TORCH_DTYPE: Final[torch.dtype] = torch.float16 if args.torch_dtype == "float16" else torch.bfloat16

+    tokenizer = AutoTokenizer.from_pretrained(args.retriever_model_name_or_path)


like you did in the eval_rag script, can you just take the tokenizer from the retriever_model? Move this below the line after

retriever_model = AutoModelForSentenceEmbedding( args.retriever_model_name_or_path, tokenizer, get_peft=False, use_bnb=False )

then do

tokenizer = retriever_model.retriever_tokenizer

Let's just use the End2End model’s wrapper class. But yeah can do as you mentioned as well. But the wrapper class does everything.

I don't understand, there is no attribute retriever_tokenizer for retriever_model (unless there's some meta level extraction I can't see in the model code) and the tokenizer would be the very same I pass in, the initialization is done outside. The end2end does the initializing but AutoModelForSentenceEmbedding does not, https://github.com/arcee-ai/DALM/blob/main/dalm/models/retriever_only_base_model.py#L12

Yeah sorry it should be just the tikenizer. You're right. Feel free to change it

Ben-Epstein · 2023-09-18T11:18:18Z

dalm/eval/eval_rag.py

-    rag_model.attach_pre_trained_peft_layers(
-        args.retriever_peft_model_path, args.generator_peft_model_path, args.device
-    )
+    # rag_model.attach_pre_trained_peft_layers(


why is this commented out?

To test in case you can't find the peft layers

Left in during testing, my bad

Ben-Epstein · 2023-09-18T11:20:34Z

dalm/eval/eval_retriever_only.py

+    # TODO: ask if this is a mistake
+    # retriever_tokenizer = retriever_model.retriever_tokenizer


ah, this is what I suggested above https://github.com/arcee-ai/DALM/pull/38/files#r1328573360

Maybe i'm missing something, why is this a mistake?

No this is not. Retriever class initlaize both the model and the tokenizer.

right, but you can still use this as the tokenizer, like I said https://github.com/arcee-ai/DALM/pull/38/files#r1328573360 right?

wouldn't this tokenizer be the exact same one as
tokenizer = AutoTokenizer.from_pretrained(args.retriever_model_name_or_path) ?

dalm/eval/eval_rag.py

shamanez · 2023-09-18T12:12:43Z

dalm/eval/eval_rag.py

-    rag_model.attach_pre_trained_peft_layers(
-        args.retriever_peft_model_path, args.generator_peft_model_path, args.device
-    )
+    # rag_model.attach_pre_trained_peft_layers(


To test in case you can't find the peft layers

shamanez · 2023-09-18T12:16:08Z

dalm/eval/eval_rag.py

@@ -295,22 +301,29 @@ def get_passage_embeddings(

        # this query comes without the answer
        query = f"#query# {test_example[args.query_column_name]} #passage# {search_result_passage} #answer# "
+        queries_for_gen_eval.append(query)


Will this try to send all the prompts through the generator? If so it will easily run out of memory. We need to create some batches

Can we also evaluate the retriever as a batch rather than a single query per time?

Will this try to send all the prompts through the generator? If so it will easily run out of memory. We need to create some batches

Fixed bcfb79c

Can we also evaluate the retriever as a batch rather than a single query per time?

I'm not sure how to do that currently. Searching the hnsw index, given the way get_nearest_neighbours is written, it only takes 1 at a time.

I think I've gotten somewhere with this

Yeah atm, we only take the top-1 passage and check the EM of the answer. But it could be more as well.

shamanez · 2023-09-18T12:20:24Z

dalm/eval/eval_retriever_only.py

@@ -104,9 +104,11 @@ def main() -> None:
    args = parse_args()
    SELECTED_TORCH_DTYPE: Final[torch.dtype] = torch.float16 if args.torch_dtype == "float16" else torch.bfloat16

+    tokenizer = AutoTokenizer.from_pretrained(args.retriever_model_name_or_path)


Let's just use the End2End model’s wrapper class. But yeah can do as you mentioned as well. But the wrapper class does everything.

shamanez · 2023-09-18T12:21:47Z

dalm/eval/eval_retriever_only.py

+    # TODO: ask if this is a mistake
+    # retriever_tokenizer = retriever_model.retriever_tokenizer


No this is not. Retriever class initlaize both the model and the tokenizer.

dalm/eval/eval_retriever_only.py

dalm/eval/eval_rag.py

…oint

…/opti-eval

shamanez · 2023-09-18T21:52:16Z

dalm/eval/eval_retriever_only.py

@@ -219,6 +219,25 @@ def get_passage_embeddings(

    print("Evaluation start")

+    # ruff:noqa
+    def my_collate_fn(batch: List[Dict[str, torch.Tensor | str]]) -> Dict[str, torch.Tensor | List[str]]:


Beautiful.. if we use the default collate function we will loose all the text outputs that need to compute the precision and the accuracy .. let’s add this to the e2e as well.

…s are effects of this change

…eval_rag.py

shamanez

looks good

shamanez and others added 8 commits September 18, 2023 11:47

Merge pull request #35 from arcee-ai/eval_script_optimization

2456e42

Update eval_with_hnsw.py

batch predictions

38ef6c9

remove pipeline

bcf148c

todo

9f35f2b

Merge remote-tracking branch 'origin/eval_script_optimization' into c…

f764ebf

…hore/opti-eval

Corrections to make local testing work

084beca

Correct model variable name

dc561ab

Corrections for e2e eval script post trying to run locally

7dc7b65

metric-space mentioned this pull request Sep 18, 2023

Modified the evaluation scripts for the clarity #37

Merged

metric-space requested a review from shamanez September 18, 2023 06:55

Ben-Epstein commented Sep 18, 2023

View reviewed changes

shamanez requested changes Sep 18, 2023

View reviewed changes

SachiraKuruppu reviewed Sep 18, 2023

View reviewed changes

dalm/eval/eval_rag.py Outdated Show resolved Hide resolved

Ben-Epstein added 5 commits September 18, 2023 08:52

batch decode

8765963

rename function

6907d16

do generation in batches of 16 for eval

bcfb79c

small cleanup on context manager

0899d45

fix preprocess function in eval

3ff929a

Ben-Epstein requested review from shamanez and SachiraKuruppu September 18, 2023 13:12

metric-space added 2 commits September 18, 2023 17:25

Optimize eval script to take in data as batches at main metric calc p…

4968143

…oint

Merge branch 'chore/opti-eval' of github.com:arcee-ai/DALM into chore…

47614e1

…/opti-eval

shamanez approved these changes Sep 18, 2023

View reviewed changes

metric-space added 5 commits September 18, 2023 18:30

AutoModelForSentenceEmbedding now initializes tokenizer. Other change…

38c63af

…s are effects of this change

Remove explicit tokenizer initialzation from train_retriever script

48d3d90

Make mixed_data_collator a util and considate new batch eval loop in …

c271913

…eval_rag.py

Add query_batch_size as a cli param

5206b92

Add readme to eval section

2cc6a72

shamanez approved these changes Sep 19, 2023

View reviewed changes

README name correction

58dc023

Ben-Epstein merged commit 8d3602c into eval_script_optimization Sep 19, 2023

Ben-Epstein deleted the chore/opti-eval branch September 19, 2023 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore/opti eval #38

Chore/opti eval #38

Ben-Epstein commented Sep 18, 2023

Ben-Epstein Sep 18, 2023

shamanez Sep 18, 2023

metric-space Sep 18, 2023 •

edited

Loading

shamanez Sep 18, 2023

Ben-Epstein Sep 18, 2023

shamanez Sep 18, 2023

metric-space Sep 18, 2023

Ben-Epstein Sep 18, 2023

shamanez Sep 18, 2023

Ben-Epstein Sep 18, 2023

shamanez Sep 18, 2023

shamanez Sep 18, 2023

shamanez Sep 18, 2023

shamanez Sep 18, 2023

Ben-Epstein Sep 18, 2023 •

edited

Loading

metric-space Sep 18, 2023

shamanez Sep 18, 2023

shamanez Sep 18, 2023

shamanez Sep 18, 2023

shamanez Sep 18, 2023

shamanez left a comment

		# TODO: ask if this is a mistake
		# retriever_tokenizer = retriever_model.retriever_tokenizer

Chore/opti eval #38

Chore/opti eval #38

Conversation

Ben-Epstein commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metric-space Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ben-Epstein Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamanez left a comment

Choose a reason for hiding this comment

metric-space Sep 18, 2023 •

edited

Loading

Ben-Epstein Sep 18, 2023 •

edited

Loading