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

Fix missing args in tfe.stats.RunningVariance.from_shape #1622

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

chrism0dwk
Copy link

Both the event_ndims and name arguments of the class method tfp.experimental.stats.RunningVariance.from_shape were missing, causing TypeError: from_example() got an unexpected keyword argument 'event_ndims'.

Both the `event_ndims` and `name` arguments of the class method
`tfp.experimental.stats.RunningVariance.from_shape` were missing,
causing `TypeError: from_example() got an unexpected keyword argument
'event_ndims'`.
@chrism0dwk
Copy link
Author

N.B. this fix sorts out the same TypeError when using RunningVariance.from_example().

Chris Jewell added 2 commits September 22, 2022 18:00
`tfp.experimental.stats.RunningVariance.variance` method
returned the whole underlying covariance matrix.  This change
extracts and returns just the diagonal.

This should probably be a temporary fix until a more efficient
variance-only algorithm is constructed.
@SiegeLordEx
Copy link
Member

How did you get the original error? RunningVariance is expected to have event_ndims=0, because it treats each element as an independent (co)variance estimation problem.

@chrism0dwk
Copy link
Author

chrism0dwk commented Sep 22, 2022

@SiegeLordEx The original problem arose with

tfp.experimental.stats.RunningVariance.from_example(np.array([1., 1., 1., 1.]))

The fundamental issue is with the Python MRO with RunningVariance derived from RunningCovariance:

  1. RunningVariance.from_example calls RunningCovariance.from_example;
  2. RunningCovariance.from_example calls RunningVariance.from_shape;

The issue is caused because inside RunningCovariance.from_example, cls is a reference to the derived class, not (as I think the author assumed) the base class. Thus RunningCovariance.from_example assumes that RunningVariance.from_shape takes the same arguments as its own RunningCovariance.from_shape method.

Took me a while to revise my Python MRO semantics, but here's the MRE:

class Foo:
    @classmethod
    def bar(cls, baz, quz):
        print("Foo.bar cls:", cls)
        cls.quux(baz, quz)

    @classmethod
    def quux(cls, corge, grault):
        print("Foo.quux cls:", cls)

class Garply(Foo):
    @classmethod
    def bar(cls, baz):
        print("Garply.bar cls:", cls)
        super().bar(baz, quz)

    @classmethod
    def quux(cls, corge, grault):
        print("Garply.quux cls:", cls)
        super().quux(corge, grault)

If Garply.bar's signature is incompatible with Foo.bar's signature (e.g. missing an argument), then the call stack fails.

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

Successfully merging this pull request may close these issues.

None yet

2 participants