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

Simplify __sub__ for iosys models, check D operator in h2_norm #1144

Merged
merged 3 commits into from
Nov 13, 2020

Conversation

pmli
Copy link
Member

@pmli pmli commented Oct 30, 2020

It also fixes flake8 issues about long lines.

Closes #1063.

@pmli pmli added the pr:change Change in existing functionality label Oct 30, 2020
@pmli pmli added this to the 2020.2 milestone Oct 30, 2020
@pmli pmli requested a review from sdrave October 30, 2020 14:23
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.

LGTM.

You could consider failing when D_norm gets too large instead of only giving a warning.

src/pymor/models/iosys.py Outdated Show resolved Hide resolved
@pmli
Copy link
Member Author

pmli commented Oct 30, 2020

You could consider failing when D_norm gets too large instead of only giving a warning.

Then, what would be "too large"? Some default value?

@renefritze renefritze force-pushed the iosys-model-sub_ltimodel-h2 branch from eea892d to e630b36 Compare November 3, 2020 16:41
@sdrave
Copy link
Member

sdrave commented Nov 12, 2020

You could consider failing when D_norm gets too large instead of only giving a warning.

Then, what would be "too large"? Some default value?

Yes, something like that. But maybe it is not worth the extra effort. If you fix the notebook, we can also merge as is.

@pmli pmli force-pushed the iosys-model-sub_ltimodel-h2 branch from e630b36 to e454fda Compare November 13, 2020 14:48
@codecov
Copy link

codecov bot commented Nov 13, 2020

Codecov Report

Merging #1144 (e454fda) into master (2f7955a) will increase coverage by 0.28%.
The diff coverage is 71.42%.

Impacted Files Coverage Δ
src/pymor/models/iosys.py 49.78% <71.42%> (+4.72%) ⬆️
src/pymor/tools/floatcmp.py 94.73% <0.00%> (-5.27%) ⬇️
src/pymor/vectorarrays/numpy.py 84.54% <0.00%> (+0.24%) ⬆️
src/pymor/vectorarrays/list.py 82.99% <0.00%> (+0.60%) ⬆️

@pmli pmli merged commit 61baba1 into master Nov 13, 2020
@pmli pmli deleted the iosys-model-sub_ltimodel-h2 branch November 13, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:change Change in existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LTIModel.__sub__ and H2 error issues
2 participants