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

Tutorial for optimization #1205

Merged
merged 14 commits into from Dec 9, 2020
Merged

Tutorial for optimization #1205

merged 14 commits into from Dec 9, 2020

Conversation

TiKeil
Copy link
Contributor

@TiKeil TiKeil commented Dec 3, 2020

This PR contains a very detailed tutorial for optimization. The content is highly related to my talk on the online pymor school 2020.
I will rebase this PR after #1110 has been merged. So please ignore all commits except the last two and only look at this file.

@pymor/all it would be nice if you could read through it, there might be some typos or other issues.

Let me know what you think.

@TiKeil TiKeil added the pr:new-feature Introduces a new feature label Dec 3, 2020
@TiKeil TiKeil added this to the 2020.2 milestone Dec 3, 2020
src/pymor/models/interface.py Outdated Show resolved Hide resolved
src/pymortests/demos.py Outdated Show resolved Hide resolved
Copy link
Member

@HenKlei HenKlei left a comment

Choose a reason for hiding this comment

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

Very nice tutorial! Only some minor remarks from my side...

docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
@TiKeil
Copy link
Contributor Author

TiKeil commented Dec 3, 2020

Very nice tutorial! Only some minor remarks from my side...

Thanks for the very nice review. I already included all comments.

@TiKeil TiKeil force-pushed the tutorial_optimization branch 2 times, most recently from 68ac407 to a14a988 Compare December 3, 2020 13:54
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
src/pymordemos/linear_optimization.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #1205 (a040d4b) into master (4b84e1e) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted Files Coverage Δ
src/pymor/bindings/ngsolve.py 81.34% <0.00%> (-0.75%) ⬇️

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.

In general, this is a very nice tutorial! But maybe try to give a more direct and positive answer to: 'I have a PDE-constrained optimization problem, and I want to use pyMOR for speedup. What should I try as a first step?' Reading through the very long text, at the end you might get the impression that only an open problem has been presented. You should have a feeling of achievement.

docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Show resolved Hide resolved
docs/source/tutorial_optimization.rst Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
@TiKeil
Copy link
Contributor Author

TiKeil commented Dec 5, 2020

Thanks @sdrave for the very nice and detailed review. I have addressed all your points except the citations.

I think, the text in the end reads way more positive now. I will see how I can improve this more but it should be okay already.

Removing Draft mode.

@renefritze
Copy link
Member

Thanks @sdrave for the very nice and detailed review. I have addressed all your points except the citations.

I think, the text in the end reads way more positive now. I will see how I can improve this more but it should be okay already.

Looking good 👍
If you could get your last tweaks in by tomorrow evening, that would be great.

Also, can you do me a favor and also fix a typo in the mybinder box?

Please note that starting the notebook server make take a couple of minutes.

@pymor-lab-hub-bridge
Copy link

Hey @TiKeil it looks like this PR touches our CI config (and you are not in my contributor safelist), therefore I am not starting a gitlab-ci build for it and it will not be mergeable.

@TiKeil
Copy link
Contributor Author

TiKeil commented Dec 7, 2020

Thanks @sdrave for the very nice and detailed review. I have addressed all your points except the citations.
I think, the text in the end reads way more positive now. I will see how I can improve this more but it should be okay already.

Looking good +1
If you could get your last tweaks in by tomorrow evening, that would be great.

Also, can you do me a favor and also fix a typo in the mybinder box?

Please note that starting the notebook server make take a couple of minutes.

Thanks, I have fixed the typo and increased the timeout limit.

Furthermore I have added my last changes. Thus, this can be merged if there are no other objections.

@HenKlei
Copy link
Member

HenKlei commented Dec 7, 2020

You could also add the references to the bibliography file.

Copy link
Member

@renefritze renefritze left a comment

Choose a reason for hiding this comment

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

Greenlight for CI again

@TiKeil
Copy link
Contributor Author

TiKeil commented Dec 7, 2020

You could also add the references to the bibliography file.

I intentionally did not do that because these references do not have anything to do with algorithms or implementational features in
pyMOR. Meaning, we do not provide any sort of content from these references. They are rather meant as "further reading" for the interested tutorial reader.

@HenKlei
Copy link
Member

HenKlei commented Dec 7, 2020

You could also add the references to the bibliography file.

I intentionally did not do that because these references do not have anything to do with algorithms or implementational features in
pyMOR. Meaning, we do not provide any sort of content from these references. They are rather meant as "further reading" for the interested tutorial reader.

Ah okay, sounds reasonable.

@TiKeil TiKeil force-pushed the tutorial_optimization branch 2 times, most recently from 1d7ce56 to 21bb834 Compare December 7, 2020 11:11
Copy link
Contributor

@ftalbrecht ftalbrecht left a comment

Choose a reason for hiding this comment

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

After fixing everything, I suggest to

  • go through the intro once and unify the number of whitespace lines between the code and the text (say one before/after code blocks and two before headlines) and
  • check the indentation of the code examples, in particular the creation of the data dicts.

docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
@TiKeil
Copy link
Contributor Author

TiKeil commented Dec 7, 2020

After fixing everything, I suggest to

* go through the intro once and unify the number of whitespace lines between the code and the text (say one before/after code blocks and two before headlines) and

* check the indentation of the code examples, in particular the creation of the data dicts.

Thanks @ftalbrecht, I have added the files accordingly. Some of your remarks were outdated already such that I could not apply all requested changes.

docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
docs/source/tutorial_optimization.rst Outdated Show resolved Hide resolved
@sdrave sdrave merged commit d954e0c into pymor:master Dec 9, 2020
2 checks passed
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