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

Don't enforce a non-empty shape on tensors #954

Closed
matthewfeickert opened this issue Jul 17, 2020 · 2 comments · Fixed by #972
Closed

Don't enforce a non-empty shape on tensors #954

matthewfeickert opened this issue Jul 17, 2020 · 2 comments · Fixed by #972
Assignees
Labels
good first issue Good for newcomers refactor A code change that neither fixes a bug nor adds a feature

Comments

@matthewfeickert
Copy link
Member

Description

As @lukasheinrich pointed out in PR #944, enforcing a shape of (1,) on tensors created from floats is probably not a good idea when none of the backends do this

>>> import pyhf
>>> import numpy as np
>>> 
>>> example = np.asarray(0.1)
>>> print(f"example {example} is a {type(example)} with shape {example.shape}")
example 0.1 is a <class 'numpy.ndarray'> with shape ()
>>> pyhf.set_backend("numpy")
>>> example = pyhf.tensorlib.astensor(0.1)
>>> print(f"example {example} is a {type(example)} with shape {example.shape}")
example [0.1] is a <class 'numpy.ndarray'> with shape (1,)

So we should make sure that we match the shape here

# make this so
assert np.asarray(0.1).shape == pyhf.tensorlib.astensor(0.1).shape
@matthewfeickert matthewfeickert added good first issue Good for newcomers refactor A code change that neither fixes a bug nor adds a feature labels Jul 17, 2020
@matthewfeickert
Copy link
Member Author

matthewfeickert commented Jul 17, 2020

It seems that we implemented this behavior in PR #413 because of MXNet. 🙃 Given that we don't support MXNet anymore I think there's no reason that we shouldn't implement @lukasheinrich's suggestion, as I think Issue #288 won't pop up again.

@matthewfeickert matthewfeickert changed the title Don't enforce a non-zero shape on tensors Don't enforce a non-empty shape on tensors Jul 17, 2020
@matthewfeickert matthewfeickert self-assigned this Jul 17, 2020
@matthewfeickert
Copy link
Member Author

@lukasheinrich As it affects PR #972, you would agree that pyhf.infer.test_statistics.qmu should return a () scalar and not a (1,) tensor, yes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant