facebookresearch / facebookresearch/fairo

refactor memory

Open
#982 7 comments 0 reactions 2 assignees Claimed by @kavyasrinet View on GitHub
Dominant language
Jupyter Notebook
Stars
929
Forks
123
PR merge metrics
No merged PRs in 30d

Description

## Type of Issue

Select the type of issue:
- [ ] Bug report (to report a bug)
- [ ] Feature request (to request an additional feature)
- [x] Tracker (I am just using this as a tracker)
- [x] Refactor request
- [ ] Documentation Ask

## Description

There are a large number of methods in the main memory that should be moved to be class methods of the memory nodes. The behavior of the methods should not change, just their location in the codebase. If the memory node files get too big, we can split these up. When this is done, none of the memory-node specific methods (e.g. ~line 370 to line 940 in sql_memory.py) should reside in sql_memory; ditto for mc_memory.py etc. In addition, many of these methods are overly specific, and should be removed altogether. For these removed method, mentions in the code should use other more general methods, favoring basic_search as much as possible. If there is a search necessary amongst the deleted methods that cannot be done using basic search, make an issue to add that functionality.

We should register the memory nodes classes in the main memory, and continue to allow using the methods by agent_memory.nodes[node_key].method(agent_memory, *args, **args)

## Move to class methods of memory nodes:
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L412
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L430
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L453
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L472
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L500
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L590
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L600
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L609
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L656

In craftassist:
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L420
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L431
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L457
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L478
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L643
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L665
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L769

## to delete:
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L370
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L573
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L582
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L590
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L635
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L643
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L669
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L684
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L694
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L706
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L714
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L726
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/sql_memory.py#L734

In craftassist:

- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L402
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L526
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L539
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L556
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L560
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L568
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L571
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L579
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L592
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L596
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L627
- [x] https://github.com/facebookresearch/fairo/blob/8fcc711866af63ee5dd1839a0d356f2208ad54d5/droidlet/memory/craftassist/mc_memory.py#L830

## skipped things
For now have left out Tasks; will do those separately as refactor is less simple than just move/delete. For each move/delete, one needs to make sure all the mentions of that method are properly replaced (and in the case of deletes, this may require a non-mechanical change). Also left out item stacks in craftassist, these should be rewritten

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.