isi-vista / adam

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

Teaching contrastive learner fails to retrieve meaning when apprentice has no primary template for a scene #1138

Closed spigo900 closed 2 years ago

spigo900 commented 2 years ago

When its apprentice has no primary template for a scene, the contrastive learner fails to retrieve a meaning for the scene leading to a crash.

This is a problem because it happens with the subset learner in the simulated setting. I had it happen with contrastive object learning on the M5 objects curriculum. The "problem object" is sofa. We end up with two hypotheses that differ only in their stroke graph. Specifically, one has a single-point stroke graph and the other has a two-linked-points stroke graph. This is stable -- the two-points hypothesis do not seem to be contradicted/intersected out. Now, abstractly, when you ask the subset learner for its concept to template map, it includes only those hypotheses it's sure of, i.e. where it has only one. So in this situation the contrastive learner tries to get a hypothesis for sofa and match/update that to the sofa scene, but the subset learner won't return any such hypothesis because we have two for sofa.

It is simple enough to work around this by setting beam_size: 1, which is what I did. This seems a bit hacky, however.

Fix: Change the contrastive learner to handle multiple hypothesis

The obvious fix is to return some hypothesis for these scenes. However, that complicates contrastive learning and the method we use to propose pattern updates. Both currently assume each concept of interest has only one candidate meaning.

So, the contrastive learner would have to change. We would have to contrast each candidate meaning vs. each candidate meaning instead of assuming we have 1 pattern for each concept. I suspect there would be some plumbing to do to make contrastive learning work this way.

And we would have to change how we propose updated hypotheses. Currently, we propose updates by concept, and demand that each concept to be updated have only one hypothesis. To fix this, maybe instead of proposing a simple one-level dict, we could propose a nested dict: Mapping[Concept, Mapping[PerceptionGraphPattern, PerceptionGraphPattern]]. Patterns are hashable so this should work, though it is a little ugly.

Returning to the obvious part -- we should return some hypothesis -- we would want to add a new method to template learners at the same level as concepts_to_patterns/self._primary_templates. Here are two possible variations on what to return:

  1. return the set of all the hypotheses, self._concept_to_hypotheses[concept], or
  2. return only the "leading" hypothesis, self._concept_to_hypotheses[concept][0].

I'm inclined to do things as in (1) i.e. return all the hypotheses. I don't see a strong reason not to update everything -- we shouldn't have so many patterns that speed is a problem.

lichtefeld commented 2 years ago

I agree with your proposed fix. My only thought is perhaps rather than 'all' we limit to the top N? Given the number of contrastive matches that have to run now scale as N*M (N and M being the lengths of the hypotheses for each concept) we may want the ability to limit in the future. This also seems like the type of limit which is easier to add now when we're adding it to one learner than decide sometime in the future "we'd like to limit this" and need to update several learners to accept the new parameter.

spigo900 commented 2 years ago

@lichtefeld I agree, I think it makes sense to include an optional keyword argument on the new method which argument limits the number of hypotheses returned. That would mean if we need to limit things in the future we can add a parameter to control the number of hypotheses updated and we'll only need to change the contrastive learning code to do it. Yes, good idea.