clawpack / riemann_book

An interactive book about the Riemann problem for hyperbolic PDEs, using Jupyter notebooks.
BSD 3-Clause "New" or "Revised" License
262 stars 95 forks source link

Comments and suggestions for Part II #200

Open maojrs opened 5 years ago

maojrs commented 5 years ago

@rjleveque @ketch I went over the second part, took me a bit longer than expected. I think it is overall in good shape, but of course there are a couple of thinks we could still improve. My comments/suggestions follow:

Chapter 11:

Chapter 12:

Chapter 13:

Chapter 14:

Chapter 15:

I can contribute by trying improvements on Burgers approximate, adding the HLLC solver to the Euler compare and adding a tab to choose density/momentum or energy in the interacts of Chapter 15. If this is ok with you since you wrote these chapters, I am happy to implement these, so please let me know.

ketch commented 5 years ago

Thanks for the suggestions. In the future let's stick to referring to chapters by name rather than number, since the numbering is more ephemeral (indeed, the preface should really be front matter so all the chapter numbers will be reduced by one).

Sec 11.1, 4th paragraph: State explicitly what are R and \Lambda. Or at least cite Chapter 4, where we first introduce this diagonalization.

I added the phrase "eigenvalue decomposition" before this is introduced.

Sec 11.1, end: Should we cite notebooks not in printed version of the book like acoustics in heterogeneous media?

I think not; that notebook hasn't been updated for quite some time. I removed the italicized comment to ourselves.

Before Sec 11.2 and Sec 11.3: maybe add a small paragraph saying that most approximate (if not all) Riemann solvers fall within two categories: linearized Riemann solvers and two (or more) wave solvers.

Depending on your definition of "two or more wave solvers", this claim is either trivially true or (in my opinion) not true. We can discuss it further if you disagree.

Sec 11.3: Mention that in some special cases additional waves can be added to the two-wave solvers (as is the case with the HLLC)

I think that would be fine, especially if you want to add HLLC to the book.

Chapter 12

I'm hesitant to spend more time on the HLL solver here since I'm pretty sure nobody has ever actually used a two-wave solver for Burgers, and it seems a bit foolish to do so for a scalar equation. Showing the exact solution and the Roe solution would be nice for the transonic rarefaction case (in the case of a shock I think all three will be indistinguishable). Please go ahead and add it.

Sec 13.1.1, first paragraph: Maybe say explicitly that the averages must be chose to satisfy Property 3.

I don't understand. Doesn't it say that already?

Sec 13.1.2, after first code section and before examples: Add subsection for Roe solver examples and maybe a brief description before each example.

Sec 13.2.2 examples, Same as before, maybe add a brief desription before each example: Dam-break, isolated 2-shock, transonic rarefaction

Good ideas; done.

The interact from the last two code cells in the notebook doesn't work.

It works for me. I'm happy to help you debug.

Second paragraph: When mentioning the Energy is given by the relation 14.4, maybe also add the ideal gas equation of state for reference.

Good idea.

Sec 14.1, first paragraph. Reference as in Chapter not working in pdf. Maybe this is already fixed in latest version of make_pdf.

Should be fixed now.

Sec 14.2 HLLE Solver: Many parts uses HLL instead of HLLE. I know it is basically the same, but maybe somewhat confusing to have the two acronyms. Maybe change all to HLLE since we are using the HLLE choice of wave speed.

Good catch. I changed it to HLLE almost everywhere. Left one instance that I think should remain as HLL.

Maybe add the HLLC/HLLEC solver as an example of a three wave HLL solver. I am happy to do this.

That would be great!

adding a tab to choose density/momentum or energy in the interacts of Chapter 15

That would also be great.

I've implemented most of your suggestions in #202. Thanks again.

maojrs commented 5 years ago

@ketch Thank you for going over these. The interacts in the end of shallow_water approximate were not working because I needed to update clawpack, so it is all good now. On my side I will work on the following then:

ketch commented 5 years ago

That sounds great.