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

Tutorials for SNES and TS #15067

Closed

Conversation

stefanozampini
Copy link
Contributor

Depends on #15066 #15065

@stefanozampini
Copy link
Contributor Author

@luca-heltai Please assign reviewers

@luca-heltai
Copy link
Member

luca-heltai commented Apr 11, 2023

Since these depend on #15065 and #15066, I've marked it as WIP, as it won't compile without those two PRs, however I think it is worth it to start reviewing these now, since tutorials usually take longer to review.

I especially like the documentation on how to monitor SNES from the command line! Thanks for this contribution @stefanozampini! I really appreciate it!

examples/step-67/step-67.cc Outdated Show resolved Hide resolved
examples/step-67/step-67.cc Outdated Show resolved Hide resolved
examples/step-67/step-67.cc Outdated Show resolved Hide resolved
examples/step-67/step-67.cc Outdated Show resolved Hide resolved
@stefanozampini stefanozampini changed the title Stefanozampini/tutorials Tutorials for SNES and TS Apr 22, 2023
@stefanozampini
Copy link
Contributor Author

@bangerth This is now ready to be reviewed

@luca-heltai
Copy link
Member

@stefanozampini, as we discussed, I think there should be a bit of rework in the tutorials structure in deal.II before going in the direction you suggest in this PR.

In particular, we don't have a general consensus on how to tackle all these cases in which a tutorial is a minor change w.r.t. to an existing one, and your case (similarly to the NOX solver, that was closed some time ago for the same reason) is an example.

For the TimeStepper one, instead, adding it to step-67 (which already tackles a lot of new topics in the matrix free framework) does not seem to be the best solution from the pedagogical point of view. I think we reached a consensus on adding the TimeStepper class to step-26, whose goal was to teach time dependent problems, and to add some comments to the existing non-linear tutorial that uses sundials (step-77) showing what to change in order to use SNES instead (since the interfaces are almost identical).

Would this be ok from your point of view? We can work out the details of step-26 together if you want.

@stefanozampini
Copy link
Contributor Author

@stefanozampini, as we discussed, I think there should be a bit of rework in the tutorials structure in deal.II before going in the direction you suggest in this PR.

In particular, we don't have a general consensus on how to tackle all these cases in which a tutorial is a minor change w.r.t. to an existing one, and your case (similarly to the NOX solver, that was closed some time ago for the same reason) is an example.

For the TimeStepper one, instead, adding it to step-67 (which already tackles a lot of new topics in the matrix free framework) does not seem to be the best solution from the pedagogical point of view. I think we reached a consensus on adding the TimeStepper class to step-26, whose goal was to teach time dependent problems, and to add some comments to the existing non-linear tutorial that uses sundials (step-77) showing what to change in order to use SNES instead (since the interfaces are almost identical).

Would this be ok from your point of view? We can work out the details of step-26 together if you want.

Your wish is my command. The step-67 addition was to just show how easy one can switch from one implementation to another but I can drop it, no problem.

Regarding step-86: it really contains all the important information a user must know about using the SNES solver and how to deal with non-homogenous boundary conditions. I put some effort into the documentation to describe what can be done with the class in terms of solver experimentations, and I think this should be instead considered to be accepted. I believe most of the code can be reused for NOX and I don't see why the tutorial cannot be extended to NOX too.

@stefanozampini
Copy link
Contributor Author

A note aside: are the PETSc tests run in the github-linux configurations? petsc_ts_02 is supposed to fail here because I forgot to update the code. Now that PETSc classes are more reliable, wouldn't be better to run all the PETSc tests also in some of these configurations?

@bangerth
Copy link
Member

bangerth commented May 3, 2023

@stefanozampini We had a conversation this morning on the developer call on how we wanted to proceed with these variations of tutorial programs. I think we were generally not excited about having multiple minor variations of the same program add to the total number of tutorial programs -- these might serve as good starting points, but they do not actually teach orthogonal concepts as we would like them to, and they will also be difficult to keep in sync.

What I think we settled on was the following idea:

  • Each of these variations should be part of the test suite. For the SNES variation of step-77 you wrote, I put that program into the test suite in Add the PETSc SNES version of step-77 as a test. #15173.
  • We would then mention the variations in the "Possibilities for extensions" section of each of the tutorial programs. This may reference parts of the code that is in the test suite now. I can take most of your text and put that there if you want.

Would that sound like a reasonable approach you could live with?

@stefanozampini
Copy link
Contributor Author

@stefanozampini We had a conversation this morning on the developer call on how we wanted to proceed with these variations of tutorial programs. I think we were generally not excited about having multiple minor variations of the same program add to the total number of tutorial programs -- these might serve as good starting points, but they do not actually teach orthogonal concepts as we would like them to, and they will also be difficult to keep in sync.

What I think we settled on was the following idea:

* Each of these variations should be part of the test suite. For the SNES variation of step-77 you wrote, I put that program into the test suite in [Add the PETSc SNES version of step-77 as a test. #15173](https://github.com/dealii/dealii/pull/15173).

* We would then mention the variations in the "Possibilities for extensions" section of each of the tutorial programs. This may reference parts of the code that is in the test suite now. I can take most of your text and put that there if you want.

Would that sound like a reasonable approach you could live with?

Doing that, I believe there's a lot of value in NOX and SNES that will go unnoticed for the deal.II users that want to solve complicated nonlinear problems. Look at the discussion on the results, for example, for SNES. I would like to see another one for NOX, too, all in a single tutorial, so that users can go through them, understand how to code a solver-agnostic interface, and experiment.

Yes, the main business of deal.II is into the discretization of complicated systems, but at some point you need to solve them

@bangerth
Copy link
Member

bangerth commented May 4, 2023

My plan was to basically take all of the results section you wrote and include it into the results section of step-77. I think you did a fantastic job documenting the many options SNES provides, and I want to preserve that. I just don't want a separate program that is for all practical purposes a copy of step-77, with an essentially identical introduction.

How about I take what you wrote and propose a patch we can look at?

(For the ODE example: As others have mentioned above, I think that we do want a separate example program, but one that uses a simpler case to start with. Everyone's preference seems to be step-26.)

@bangerth bangerth added this to the Release 9.5 milestone May 9, 2023
@bangerth
Copy link
Member

bangerth commented Jun 5, 2023

@stefanozampini Ping?

@stefanozampini
Copy link
Contributor Author

@stefanozampini Ping?

Ping for what if I may ask?

@bangerth
Copy link
Member

bangerth commented Jun 5, 2023

The discussion in my last comment.

@stefanozampini
Copy link
Contributor Author

The discussion in my last comment.

Oh sorry, I missed it. Yes, go ahead and do what you think is best

@bangerth
Copy link
Member

The discussion of the modification of step-77 to use SNES is now in #15410.

The modification of step-67 to use PETSc TS is a separate matter, and will have to be dealt with at a later time. We do want tutorial programs to show how to use external time steppers, but perhaps not a small modification of the already rather complicated step-67. I think it might be nice to turn the modification of step-67 @stefanozampini has come up with into a test suite program.

@bangerth
Copy link
Member

I turned the PETSc TS version of step-67 into a test in #15420. Once that is merged, I will close this PR since we have used all parts in some way or other.

There remains the issue of wanting to write a tutorial program that uses PETSc TS. This is on my list, hopefully, for the hackathon.

@bangerth
Copy link
Member

bangerth commented Jul 2, 2023

The PETSc TS tutorial is tracked in #15540 .

@bangerth bangerth closed this Jul 2, 2023
@stefanozampini stefanozampini deleted the stefanozampini/tutorials branch July 18, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants