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

should append complain if output ndarray has wrong shape? #445

Closed
djerius opened this issue Jul 22, 2023 · 3 comments
Closed

should append complain if output ndarray has wrong shape? #445

djerius opened this issue Jul 22, 2023 · 3 comments

Comments

@djerius
Copy link
Contributor

djerius commented Jul 22, 2023

PDL 2.084

While rehabilitating the tests for PDL::Primitive, I came across this behavior:

This works as expected:

pdl> $x = zeroes(2); p append(pdl([2]),pdl([3]), $x);
[2 3]

But this works partially, without any complaints:

pdl> $x = zeroes(1); p append(pdl([2]),pdl([3]), $x);
[3]
@djerius
Copy link
Contributor Author

djerius commented Jul 22, 2023

More research:

pdl> append( pdl(1), pdl(2), zeroes(3) )
Error in PDL::Primitive::append: parameter 'c' index mn size 2, but ndarray dim has size 3
 at primitive.pd line 1513.
 
pdl> append( pdl([1,1]), pdl(2), zeroes(3) )

pdl> append( pdl([1,1]), pdl(2), zeroes(4) )
Error in PDL::Primitive::append: parameter 'c' index mn size 3, but ndarray dim has size 4

Looks like it only complains if the output is too big, not if its too small

@djerius
Copy link
Contributor Author

djerius commented Jul 24, 2023

The message is emitted by pdl_dim_checks, but I'm not familiar enough with the code to know if this is just an issue with append not being strict enough or a larger issue in pdl_dim_checks

@mohawk2
Copy link
Member

mohawk2 commented Jan 30, 2024

It would also have rejected if the output size were too small, but not 1. It was allowing a dim size of 1, which is fine for inputs since that will match any size. The fix here is to allow non-matching size of 1 for inputs, but not to allow that for outputs.

Thanks for the report.

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