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

rldvec relies on being given correctly-sized but padded vectors ndarray #460

Closed
mohawk2 opened this issue Jan 14, 2024 · 3 comments
Closed

Comments

@mohawk2
Copy link
Member

mohawk2 commented Jan 14, 2024

I was just re-reading the code for rldvec, and its Pars are: indx a(N); b(M,N); [o]c(M,N).

@moocow-the-bovine The output 2nd dim of N looks to me like it relies on the inputs having the right size, including a number of zeroes on the end of a. Am I missing something? If I'm right, then I think the output N needs to be another dim (perhaps N2), and it might as well be calculated by a RedoDimsCode rather than with Perl code.

@moocow-the-bovine
Copy link
Contributor

@mohawk2 I'm not sure I understand the question... I looked over the rldvec code in slices.pd myself again just now, but aside from a documentation typo (the $c() at line 1249 should be $b()), nothing is jumping out at me.

Maybe you're referring to the (hack-y) if (!defined($c)) { ... } block in the PMCode at lines 1223-1229 ? That looks like something I might have written trying to simulate RedoDimsCode, but I don't recall what exactly I was thinking at the time (and I don't believe I ever actually wound up using rldvec() for anything productive either), so...

Am I missing something?

Probably not.

I think the output N needs to be another dim

Comparing the type signature of rldvec to that for rld (viz. $a(n); $b(n); [o]$c(m)), I think you're probably right.

@mohawk2
Copy link
Member Author

mohawk2 commented Jan 14, 2024

Thank you for the quick reply! I think the same logic applies wherever there is a zeroes being used to provide an input, the calculation should be done rather than just relying on the inputs for this. Being able to provide nulls is a really nice feature, let's use it!

@mohawk2
Copy link
Member Author

mohawk2 commented Feb 25, 2024

The linked commit executes this, so closing. Thank you for the help!

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