Update MAP expected values to match map_at_k in notebooks - #2372
Open
miguelgfierro wants to merge 6 commits into
Open
Update MAP expected values to match map_at_k in notebooks#2372miguelgfierro wants to merge 6 commits into
miguelgfierro wants to merge 6 commits into
Conversation
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
miguelgfierro
requested review from
SimonYansenZhao,
anargyri,
loomlike and
wav8k
as code owners
September 4, 2026 16:32
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The notebooks were migrated from
maptomap_at_kand re-executed, but the tests were not updated. So the tests compare amap_at_kresult against an oldmapexpectation, and fail.map_at_kdivides bymin(k, n_relevant)instead ofn_relevant, so it is always greater than or equal tomap. That is why every value below goes up.This PR only moves the test expectations to the values the notebooks already produce.
test_sar_single_node_smoketest_sar_single_node_functionaltest_sar_single_node_functionaltest_baseline_deep_dive_smoketest_baseline_deep_dive_functionaltest_cornac_bivae_smokeEach value comes from running the notebook with the parameters the test uses. The 1m and 10m rows have no notebook value to cite because those notebooks are committed at 100k.
Only MAP moved. Every other metric in these tests still holds and is left untouched.
BPR is fixed separately in #2371, because it changed for a different reason.
Benefit
Unblocks
group_cpu_002andgroup_cpu_003of the CPU nightly, which has had no green run since 2026-05-04. A real ranking regression will now surface instead of hiding behind an assertion that already fails.Risk
test_vw_deep_dive_smokeandtest_vw_deep_dive_functionalare skipped (VW pip package has installation incompatibilities), so they do not affect the nightly. Their MAP expectations are stale for the same reason as the rest of this PR, and so is the committed MAP of 0.012535 in vowpal_wabbit_deep_dive.ipynb, which predates the redefinition. Neither can be corrected until VW installs again:vowpalwabbit>=8.9.0,<9has no cp310 or cp311 wheels, so the notebook cannot be re-run on a supported Python.EPOCHS, so their notebook outputs are not comparable and correct values need GPU runs.test_cornac_bivae_smokeis the only one whose parameters match, and it is included here.Checklist:
stagingbranch AND NOT TOmainBRANCH.