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

RFC: proposed new array assignment shape matching rule #5226

Closed
wants to merge 8 commits into from

Conversation

JeffBezanson
Copy link
Member

would fix #4048, #4383

This rule ignores singleton dimensions, and allows the last dimension of one side to match all trailing dimensions of the other.

The logic to implement this is pretty complicated, and I hope some code simplifications can be found. Indexing-related code always makes my brain freeze.

Right now, we don't even have a consistent rule. For example the left-hand side A[1, 1:n] permits different right-hand sides than A[1:1, 1:n]. And 1-d right-hand sides are special-cased, but the general version is to allow the last dimension of the RHS to match trailing indexes.

A rule like this is safe regardless of how we decide to change the number of dimensions returned by getindex.

EDIT: Follow-on work still needed to use this rule everywhere. Tests also needed.

…#4383

this rule ignores singleton dimensions, and allows the last dimension of
one side to match all trailing dimensions of the other.
@StefanKarpinski
Copy link
Member

I agree that the indexing stuff is paralyzingly complex. Needs a serious overhaul.

@timholy
Copy link
Member

timholy commented Dec 26, 2013

This is a nicely-isolated change to the indexing code, and is an improvement worth having. Agreed that tests would be helpful, although you'll get much of that just from the functions that depend on this.

@mschauer
Copy link
Contributor

I was looking at the problem a bit. Isn't the core logic completely symmetric? Then the symmetries can be exploited a bit. In a stylized form, where ii and jj play the role of size(X,i) and length(I[i]):

function f(I,J)
    li = length(I)
    lj = length(J)
    i = 1
    j = 1
    while true
        ii = I[i]
        jj = J[j]
        if i == li || j == lj 
            while j < lj
                j += 1
                jj *= J[j]
            end
            while i < li
                i += 1
                ii *= I[i]
            end
            if ii != jj
                error("mismatch")
            end
            return true
        end
        if ii == jj
            i += 1
            j += 1
        elseif ii == 1
            i += 1
        elseif jj == 1
            j += 1
        else
            error("mismatch")
        end
    end
end

@timholy
Copy link
Member

timholy commented Dec 28, 2013

I too had a sense that was probably true; thanks for taking the time to check that suspicion.

I'm sure you know this, but in production code for reasons of performance we need to retain the form of arguments that Jeff used.

block REPL I/O task while evaluating input
…#4383

this rule ignores singleton dimensions, and allows the last dimension of
one side to match all trailing dimensions of the other.
also reduce duplication of the related error messages
@mschauer
Copy link
Contributor

Do we need to check li == 0 or lj == 0?

@JeffBezanson
Copy link
Member Author

That seems to get magically handled by size(X,i) always giving 1 for a 0-d array, and there is a specialized method for the case of zero indexes.

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.

array assignment a[1,:,:] = b fails even when b is the correct size
4 participants