isi-vista / adam

Abduction to Demonstrate an Articulate Machine
MIT License
11 stars 3 forks source link

Add new nodes to ADAM perception graph #1198

Closed boyleconnor closed 1 year ago

boyleconnor commented 1 year ago

I defined and added three new top-level nodes to the ADAM perception graph, size, relative_size, and relative_distance. I also changed one of the feature extraction steps of ADAM to output a dict/object for the relative_size feature instead of the list/array (which was harder to read as I originally coded it; you had to infer the relevant object_name instead of it being stored explicitly).

This PR is definitely not done yet, as it doesn't handle the touching feature, but I want to get eyes on this ASAP.

I also wanted to hear from @lichtefeld and/or @spigo900 whether there's anything downstream of this that I need to change to make sure that these new nodes in the perception graph are getting ingested properly.

boyleconnor commented 1 year ago

I should mention that I'm getting a bunch of MyPy issues that I don't think are related to my code:

Non-relevant MyPy errors ``` adam/experiment/log_experiment.py:547: error: Argument "attribute_learner" to "SymbolicIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:548: error: Argument "relation_learner" to "SymbolicIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:549: error: Argument "action_learner" to "SymbolicIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:550: error: Argument "functional_learner" to "SymbolicIntegratedTemplateLearner" has incompatible type "Optional[FunctionalLearner]"; expected "FunctionalLearner" adam/experiment/log_experiment.py:553: error: Argument "generics_learner" to "SymbolicIntegratedTemplateLearner" has incompatible type "Optional[SimpleGenericsLearner]"; expected "SimpleGenericsLearner" adam/experiment/log_experiment.py:556: error: Argument "plural_learner" to "SymbolicIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:606: error: Argument "attribute_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:607: error: Argument "relation_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:608: error: Argument "action_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:609: error: Argument "functional_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[FunctionalLearner]"; expected "FunctionalLearner" adam/experiment/log_experiment.py:612: error: Argument "generics_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[SimpleGenericsLearner]"; expected "SimpleGenericsLearner" adam/experiment/log_experiment.py:615: error: Argument "plural_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:617: error: Argument "affordance_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[TemplateLearner]"; expected "TemplateLearner" adam/experiment/log_experiment.py:618: error: Argument "mapping_affordance_learner" to "SimulatedIntegratedTemplateLearner" has incompatible type "Optional[MappingAffordanceLearner]"; expected "MappingAffordanceLearner" ```

Not sure what to do about these

spigo900 commented 1 year ago

@boyleconnor Do you get those MyPy errors when running make mypy? I'm not able to reproduce with that command locally.

spigo900 commented 1 year ago

Aha, it happens when I upgrade to 0.982. 0.942 (my original version) and 0.950 (locked) are too old to get the error. It looks like those log_experiment warnings are spurious, because the arguments it's complaining about should have optional type and are being assigned from an optional type, but I'm not sure why they're happening.

lichtefeld commented 1 year ago

@boyleconnor Just #type: ignore them. mypy is improved beyond some of ADAM's original code base and sometimes our usage of inheritance still causes mypy issues. Given the time sink it would probably be to correctly update everything across the library its not worth it.