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

Accept None in pylops.Block #110

Open
RendersJens opened this issue Oct 12, 2019 · 3 comments
Open

Accept None in pylops.Block #110

RendersJens opened this issue Oct 12, 2019 · 3 comments

Comments

@RendersJens
Copy link

When creating a block operator, often some coefficients are zero. Right now, you need to use a Zero operator like:

pylops.Block([[A, A               ],
              [A, pylops.Zero(n,m)]])

while in for example scipy.sparse.bmat you can just write

bmat([[A, A   ],
      [A, None]])

which is much more expressive, and the correct dimensions of the zero block are inferred from the other blocks.

@mrava87
Copy link
Collaborator

mrava87 commented Oct 12, 2019

Hi,
Thanks for the suggestion. I need to think about how much additional checks would be required within the operator to avoid the user having to specify a Zero operator.

Note that Pylops Zero operator has pretty much no additional memory footprint, so the argument for having None is purely stylistic. I am a bit concerned that users may start using any weird combination of A and None, say:

[[A, None,], [A, None]]

which is an ill-poses definition as you can’t derive the number of columns of None given just A. And same if you have a 3x3 block operator and put all None in a column. In other words, to allow the user not having to specify a Zero operator, we then need to do many different checks, ultimately adding computational cost instead of reducing it.

I suggest we first tackle #109 and then see if this makes sense given the additional complexity it adds. We can also take a look at the inner working of the scipy method to see if we can do it in a similar fashion

@RendersJens
Copy link
Author

Yes, it is a purely stylistic issue, but still an issue (altough low priority). Indeed, the scipy bmat method is a perfect example of how it should be done. this function removes rows and colums which contain only None.

@mrava87
Copy link
Collaborator

mrava87 commented Oct 14, 2019

The main problem is that Block would need to be redesigned completely.

Now it is two lines taking advantage of the fact that a Block matrix is a vertical stack of horizontal stacks:

hblocks = [_HStack(hblock, dtype=dtype, **args_HStack) for hblock in ops]
return _VStack(hblocks, dtype=dtype, **args_VStack)

I am not that convinced of creating a much longer routine just to handle None is a good idea. Scipy bmatcannot take advantage of this property and can find out the size of None while filling the matrix, but the benefit of linear operators is that you don't need to explicitly create a matrix and to allow None this property would go away.

Unless you have any idea on how to have None while keeping the code for Block very short, I am afraid but users would need to be explicit here.

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