Skip to content

Incorrect predicate application in EagerTestSession and GraphTestSession #223

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

Open
deansher opened this issue Feb 20, 2021 · 5 comments
Open

Comments

@deansher
Copy link
Contributor

I believe this is a widespread broken pattern in EagerTestSession and GraphTestSession:

        o.asTensor()
            .scalars()
            .forEachIndexed((idx, f) -> assertTrue(predicate.test(o.asTensor().getFloat())));

Notice that f is ignored in the lambda, and o is referenced directly instead. getFloat then returns the zero'th element of the tensor.

@JimClarke5, am I right here?

@JimClarke5
Copy link
Contributor

Where are you seeing this? I cannot find it in TestSession, EagerTestSession or GraphTestSession.
It may have been something I already fixed when I refactored TestSession a few PRs ago.

The closest I can find to the above is:
TestSession: 1089-1093 in my copy.

  if (isScalar) {
      assertTrue(predicate.test(tensor.getObject()));
    } else {
      tensor.scalars().forEachIndexed((idx, s) -> assertTrue(predicate.test(s.getObject())));
    }

@deansher
Copy link
Contributor Author

Here's an example in master.

@deansher
Copy link
Contributor Author

Here's the example I originally looked at.

@JimClarke5
Copy link
Contributor

I have dramatically refactored all this, maybe my version hasn't been pushed yet.
I know the refactor is in my Layers1 branch (not pushed). I'll check Metrics2, #222, branch to see if it is in there.

@JimClarke5
Copy link
Contributor

The refactor is in Layers1, not yet pushed. I am ready to push this, but have been holding off for other PR's to be approved first.

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

No branches or pull requests

2 participants