-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
# we need at least one memory to open with... | ||
memories_to_use = random.sample(list(prefixed_memories.keys()), 1) | ||
|
||
new_obs.force_set('text', self._check_and_limit_len('\n'.join(memories_to_use))) |
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.
Why are we setting text
here?
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.
we're rebuilding the observation for the opening model; 'text'
is what the model is seeing
observations = super().observe(observation) | ||
for m in Module.dialogue_modules(): | ||
ag_obs = copy.deepcopy(observation) | ||
observations[m] = self.agents[m].observe(ag_obs) | ||
if is_opener(observation['text'], opening_memories): | ||
assert opening_memories |
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.
Question: having no memory does it make it generate an opening message anyway? I mean, if it doesn't crash the model, it may be a legitimate case to have here (?).
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.
is_opener
checks that opening_memories
is non-empty, so this assertion is not necessarily required
@@ -701,7 +725,15 @@ def _failed_messages(replies): | |||
retries += 1 | |||
n_mems = [min(1, len(obs['memories']) // 3) for obs in opening_obs] | |||
for i, o in enumerate(opening_obs): | |||
o.force_set('memories', random.sample(o['memories'], n_mems[i])) | |||
mem_indices = random.sample(range(len(o['memories'])), n_mems[i]) | |||
o.force_set( |
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.
Does this purge the existing memories if the current item in the batch doesn't have memory access this round?
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.
no, we're just playing around with the memories sent in the opening message. This is legacy logic for if the opening message is not meaningful (back when our models couldn't handle the opener)
projects/bb3/agents/utils.py
Outdated
) | ||
non_stopword_text = ' '.join(normal_tokenizer(text, include_pronouns=True)) | ||
if ( | ||
F1Metric.compute(non_stopword_memory, [non_stopword_text]).value() |
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.
nit: maybe only checking if memory_overlap_threshold > 0
? Just saving a bit on F1 compute.
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.
yeah good idea
< memory_overlap_threshold | ||
): | ||
continue | ||
if memory_overlap_threshold > 0: |
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.
👍
Patch description
Implement the following memory heuristics when selecting what memories to show the bot when crafting a response:
memory_overlap_threshold
--> if set to > 0, we block any memories that do not have this F1 overlap with the incoming textmemory_hard_block_for_n_turns
--> if set to > 0, we block memories that have been used in the last n turnsmemory_soft_block_decay_factor
--> if set to > 0, we block memories according the following probability:pr(block) = random.random() < decay_factor ** n_turns_since_used
This required an overhaul of how
memories
are stored in the agent; rather than a list of memories, we now use a dictionary mappingmemory: n_turns_since_use
Testing steps
Several more tests added