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

Comments and suggestions for Part II #200

Open
maojrs opened this issue Jul 12, 2019 · 3 comments
Open

Comments and suggestions for Part II #200

maojrs opened this issue Jul 12, 2019 · 3 comments

Comments

@maojrs
Copy link
Contributor

maojrs commented Jul 12, 2019

@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:

  • Sec 11.1, 4th paragraph: State explicitly what are R and \Lambda. Or at least cite Chapter 4, where we first introduce this diagonalization.
  • Sec 11.1, end: Should we cite notebooks not in printed version of the book like acoustics in heterogeneous media?
  • 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.
  • 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)

Chapter 12:

  • General comment: I think this notebook can be improved. It is nice we show the solutions using clawpack, but it will also be nice to show the actual solution of the approximate Riemann solver and maybe compare it with the exact solution. Specially in the case of the HLL, so it is clear it separates into three states. Also, if I am correct the HLL solver doesn't need an entropy fix for the transonic case. If so, maybe organizing the chapter differently could help make this clear. If it is ok with you, I'll give it a shot in improving this Chapter.

Chapter 13:

  • Sec 13.1.1, first paragraph: Maybe say explicitly that the averages must be chose to satisfy Property 3.
  • 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
  • The interact from the last two code cells in the notebook doesn't work.

Chapter 14:

  • Second paragraph: When mentioning the Energy is given by the relation 14.4, maybe also add the ideal gas equation of state for reference.
    Sec 14.1, first paragraph. Reference as in Chapter not working in pdf. Maybe this is already fixed in latest version of make_pdf.
    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.
    Maybe add the HLLC/HLLEC solver as an example of a three wave HLL solver. I am happy to do this.

Chapter 15:

  • In first part, maybe add a brief description before the large chunk of code of wat this code does.
  • In all the interacts, maybe we can add a tab to choose if we want to plot density, momentum or energy, so the reader can also see how the solvers perform on the other variables. I am happy to implement this.
  • Sec 15.2, High-order WENO, a longer but still brief explanation of how these methods work would be nice to have in the beggining of this Section.

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
Copy link
Member

ketch commented Jul 17, 2019

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
Copy link
Contributor Author

maojrs commented Jul 17, 2019

@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:

  • Add an interact with the exact solution and the Roe solution for the transonic rarefaction case in Burgers_approximate.
  • Add an HLLC solver in Euler_approximate and mention 3 wave cases might come up in Sec 11.3.
  • Add a tab o choose density/momentum or energy to interacts in Euler_comparisons.

@ketch
Copy link
Member

ketch commented Jul 18, 2019

That sounds great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants