Conversation
| @@ -0,0 +1,280 @@ | |||
| """SQLite-backed completion and judgement store. | |||
There was a problem hiding this comment.
I have several concerns for this:
(1) We are not using model/judge/prompt configurations in the index. Maybe this is resolved later but any change to system prompt, in my opinion, should be reflected in a key (like some unique hash maybe)?
(2) instruction_index INTEGER PRIMARY KEY Works just fine for datasets where instruction_index is unique hash key, however if it is an integer for row index at some point, (for example ELO pipeline we have), then different filterings yielding same dataset idx can query the cache even if the language for example is entirely wrong.
(3) Related to (1), UNIQUE(instruction_index, model_A, model_B) does not include the content that is judged. If for example, we use a different temperature for model_A we will still get the old judgement even if the generated output is different.
| return pd.read_sql( | ||
| "SELECT * FROM completions ORDER BY instruction_index", conn | ||
| ) | ||
| placeholders = ",".join("?" * len(indices)) |
There was a problem hiding this comment.
This will fail if len(indices) is too large.
The Limit: 32,766 parameters
If we want to get ~40.000 row this will fail. Need some chunk up loading mechanism (would allow parallization as well)
| ) -> pd.DataFrame: | ||
| # Filter to instructions not already in the shared store | ||
| if completion_store is not None: | ||
| all_indices = instructions.index.tolist() |
There was a problem hiding this comment.
We should be sure about instructions.index is never changes under the filtering steps (category/language slices etc).
| "sr", | ||
| "ar", | ||
| ] | ||
| print(f"filtering with languages: {args.languages}") |
There was a problem hiding this comment.
Maybe lets use logging.debug as we are using it before so we keep track of the logs easily (also the users)
| pairs = list( | ||
| zip(all_indices, model_A_per_row, model_B_per_row, strict=True) | ||
| ) | ||
| cached_df = judgement_store.query(model=args.model) |
There was a problem hiding this comment.
cached_df contains every cached row for the evaluated model, but it is not restricted to the keys requested by the current run before being concatenated with new_df. This can include stale or out-of-range rows when sample size, language filters, or seed change.
| ) | ||
| row_our_pos_a = np.concatenate( | ||
| [our_model_is_position_a, our_model_is_position_a] | ||
| annotations, _, prefs = judge_and_parse_prefs( |
There was a problem hiding this comment.
We give args.swap_mode, if its both, then both annotation and reversed annotation is returned. However from what I know prefs are 2m
We run this function in judge_and_parse_prefs:
def combine_swapped_prefs(prefs_ab: pd.Series, prefs_ba: pd.Series) -> pd.Series:
"""Combine swap_mode='both' prefs into one P(B wins) series: [pref_AB, 1 - pref_BA].
``prefs_ab`` are P(B wins) from the AB ordering; ``prefs_ba`` are P(B wins)
from the swapped BA ordering, so ``1 - prefs_ba`` re-orients them to the AB
frame before stacking.
"""
return pd.concat(
[prefs_ab.reset_index(drop=True), 1 - prefs_ba.reset_index(drop=True)]
).reset_index(drop=True)
prefs = combine_swapped_prefs(prefs.apply(_none_to_nan), prefs_reversed)|
|
||
|
|
||
| try: | ||
| from textual import work |
There was a problem hiding this comment.
Should we add this to pyproject.toml?
No description provided.