Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[BB2] Handle no-memories in memory_only case #4156

Merged
merged 1 commit into from
Nov 9, 2021
Merged

[BB2] Handle no-memories in memory_only case #4156

merged 1 commit into from
Nov 9, 2021

Conversation

klshuster
Copy link
Contributor

Patch description
When training with --knowledge-access-method memory_only, we send every example to access the long-term memory. If some examples within that batch do not have any memory vectors, the current access errors out.

This fix bars access to the memory_vec to be only those examples with tokenized memories; other examples are handled by adding "fake" memories, as is done in the --knowledge-access-method classify scenario.

Testing steps
Ran test_bb2.py locally:

$ pytest -k TestBB2Fid
================================================================================ test session starts ================================================================================
platform linux -- Python 3.7.9, pytest-5.3.2, py-1.10.0, pluggy-0.13.1
rootdir: /private/home/kshuster/ParlAI, inifile: pytest.ini
plugins: hydra-core-1.1.0, requests-mock-1.8.0, regressions-2.1.1, datadir-1.3.1
collected 120 items / 115 deselected / 5 selected

test_bb2.py .....                                                                                                                                                             [100%]

============================================================================= slowest 10 test durations =============================================================================
105.39s call     tests/nightly/gpu/test_bb2.py::TestBB2Fid::test_retrieval_all
79.66s call     tests/nightly/gpu/test_bb2.py::TestBB2Fid::test_retrieval_classify
77.69s call     tests/nightly/gpu/test_bb2.py::TestBB2Fid::test_retrieval_search_only
74.57s call     tests/nightly/gpu/test_bb2.py::TestBB2Fid::test_retrieval_memory_only
69.27s call     tests/nightly/gpu/test_bb2.py::TestBB2Fid::test_retrieval_none

(0.00 durations hidden.  Use -vv to show these durations.)
============================================================= 5 passed, 115 deselected, 7 warnings in 420.48s (0:07:00) =============================================================

Copy link
Contributor

@mojtaba-komeili mojtaba-komeili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and thanks for the quick fix.

@klshuster
Copy link
Contributor Author

it looks like everything is passing on ci

@klshuster klshuster merged commit 74dfeeb into main Nov 9, 2021
@klshuster klshuster deleted the ltm_fix branch November 9, 2021 21:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants