facebookresearch / facebookresearch/fairo
refactor memory
- 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
Assessment
This issue has not been assessed yet.