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

[operators] add SparseBlockOperator #889

Closed
wants to merge 1 commit into from

Conversation

ftalbrecht
Copy link
Contributor

@ftalbrecht ftalbrecht commented Feb 28, 2020

No description provided.

@ftalbrecht
Copy link
Contributor Author

Windows CI finished unexpectedly fast, @renefritze.

@@ -79,6 +79,15 @@ def action_BlockOperator(self, op):
else:
return sps.bmat(mat_blocks, format=format)

@match_class(SparseBlockOperator)
def action_SparseBlockOperator(self, op):
format = self.format
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop line

def apply_adjoint(self, V, mu=None):
assert V in self.range

V_blocks = [None for j in range(self.num_source_blocks)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> U_blocks

@codecov
Copy link

codecov bot commented Feb 28, 2020

Codecov Report

Merging #889 into master will increase coverage by 0.18%.
The diff coverage is n/a.

Copy link
Member

@sdrave sdrave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, I am not completely sure we need this in pyMOR ATM. In particular, I do not really see the practical benefit in having a sparse implementation. Also, there might be alternatives. In particular, one could extend __init__ or a have a dedicated factory method so that one can pass the blocks in some sparse format. Also, what I dislike about this approach is that it adds another incompatible type of blocked Operator I will have to special-case for in algorithms which somehow work with the block structure. I would really prefer an approach, where only the interal storage format is changed, like with NumpyMatrixOperator, maybe making blocks a property that always returns an array, or at least let SparseBlockOperator derive from BlockOperator.

We might also consider adding this to the playground. We didn't put anything there for quite a while.

@pmli
Copy link
Member

pmli commented Mar 2, 2020

It seems that pydata/sparse supports dtype=object, unlike SciPy. What do you think about using it in BlockOperator?

@sdrave sdrave added the pr:new-feature Introduces a new feature label Jul 6, 2020
@renefritze renefritze changed the base branch from master to main January 26, 2021 07:40
@renefritze renefritze added this to the 2021.2 milestone Aug 18, 2021
@sdrave sdrave modified the milestones: 2021.2, 2022.1 Dec 14, 2021
@ftalbrecht
Copy link
Contributor Author

This has been lying around for ages, without me requiring it anymore. Should we drop/delete this and start anew once someone needs this?

@pmli pmli closed this Jun 7, 2022
@TiKeil TiKeil mentioned this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:new-feature Introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants