Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent API in nupic.engine #3497

Open
oxtopus opened this issue Mar 23, 2017 · 2 comments
Open

Inconsistent API in nupic.engine #3497

oxtopus opened this issue Mar 23, 2017 · 2 comments

Comments

@oxtopus
Copy link
Contributor

oxtopus commented Mar 23, 2017

What follows is mostly mental notes re: specific implementation details of network api.

Contrary to nupic.engine.Network, which is a subclass and wrapper for the swig-generated nupic.bindings.engine_internal.Network, nupic.engine.Region is a standalone class with its own properties and behaviors independent of nupiclbindings.engine_internal.Region. Consider the following example:

region = network.addRegion("region1", "TestNode", "")
region1 = network.getRegions().getByName("region1")

You might infer from the signature of Network.addRegion() that region and region1 would be the very same instance. However, region is an instance of nupic.engine.Region whereas region1 is an instance of nupic.bindings.engine_internal.Region. The reason for this is Network.addRegion() as it is implemented at https://github.com/numenta/nupic/blob/bfa052f3d5740b3934c2dcefc566ab0906f9af2e/src/nupic/engine/__init__.py#L641-L646 calls addRegion() on the internal implementation, but then separately returns the result of self._getRegions()[name]. Network._getRegions() wraps the result of engine_internal.Network.getRegions() such that the return value is another wrapper that implements a dict-like interface.

It seems like some of the complexity can be mitigated by implementing Network.addRegion() as something like the following:

def addRegion(self, name, nodeType, nodeParams):
    """
    @doc:place_holder(Network.addRegion)
    """
    engine_internal.Network.addRegion(self, name, nodeType, nodeParams)
    return self.getRegions().getByName(name)

Or:

def addRegion(self, name, nodeType, nodeParams):
    """
    @doc:place_holder(Network.addRegion)
    """
    return engine_internal.Network.addRegion(self, name, nodeType, nodeParams)
@oxtopus oxtopus changed the title In Inconsistent API in nupic.engine Mar 24, 2017
@rhyolight
Copy link
Member

👍

@scottpurdy
Copy link
Contributor

Now that the Python classes in nupic are subclasses of the SWIG classes, we can probably safely integrate them so that the SWIG interface provides the API we want and there is only one Python class per component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants