Clear up duplicate Newton Stokes assemblers

[YiminJin]

This PR clears up duplicate assemblers for Newton solver. These assemblers include:
NewtonStokesReferenceDensityCompressibilityTerm
NewtonStokesImplicitReferenceDensityCompressibilityTerm
NewtonStokesIsentropicCompressionTerm
NewtonStokesProjectedDensityFieldTerm
These assemblers are identical to those for Stokes solver (except for NewtonStokesImplicitReferenceDensityCompressibilityTerm, in which the sign of the additional term is inverted. But I think this is a mistake).

I have checked that the modified code compiles, but I did not find tests for compressible models using Newton solver. Please check if it is necessary to test the modifications.

[gassmoeller]

Thank you for finding this!

Technically, all code functionality should be covered by tests, but that was clearly not the case here. Theoretically, now someone would need to benchmark the newton solver with compressible models, for example the benchmarks in benchmarks/compressibility_formulations. If you have a few minutes it would be great if you could compare at least one or two of the benchmarks between Picard Stokes and Newton Stokes (e.g. lateral_pipe, and vertical_pipe). The folders have scripts to run the benchmarks (run.sh), they run pretty quick, and you can collect the results (make_statistics.sh) and the results will be written into the mass_flux_error text files. I would feel better knowing that the Newton solver reaches similar accuracies as the fixed point Picard solver.

I understand if you dont have the time for that, in that case please at least copy the tests compressibility_lateral_pipe, compressibility_lateral_pipe_projected_density, compressibility_vertical_pipe, and compressibility_vertical_pipe_projected_density and modify the new tests to use the Newton solver. That way at least we have some of the features tested and will notice if something changes in the future.

Again, thanks for doing this kind of work, it improves the code significantly!

[MFraters]

Maybe to give some background, the reason there had not been any tests for compressible Newton, is because the stabilization was not implemented for the compressible equations, just for the incompressible ones. So it might have worked in some cases, but the matrix could always just become non-spd and crash. The plan was to derive and implement this stabilization afterwards, but @bangerth and I didn't get around to it.

[gassmoeller]

@YiminJin sorry for being slow to come back to this. I think we should proceed with this PR, even without tests (it is not worse than before, and like you said it even fixes a bug in one of the assemblers).

@MFraters I appreciate that you intended to extend this to compressible models, but since it is at the moment untested code and in addition duplicated code I would still be in favor of merging this PR. We can easily undo this change in the future by copying the other assemblers (with all improvements they may have gotten at that time). If you agree I would move forward with this PR.

[gassmoeller]

/rebuild

[gassmoeller]

Hi @YiminJin sorry for coming back to this so late. I still think this is worth merging. Could you please rebase the branch to the latest main branch, just to make sure all the tests are still passing? If they do, this is ready to merge.

[YiminJin]

It is I who need to apologize: I lost myself to something beyond my ability for too long a while.

This branch is rebased.

However, I found that this branch has a bug. When testing it with tests/incompressibility_iterated_stokes.prm with Stokes solver changed to "block AMG" and nonlinear solver changed to "single Advection, iterated Newton Stokes", the code throws an error in the second iteration (the first defect-correction iteration):

An error occurred in line <794> of file </home/ein/repository/aspect/source/simulator/assembly.cc> in function
    void aspect::Simulator<dim>::assemble_stokes_system() [with int dim = 2]
The violated condition was: 
    rebuild_stokes_matrix || boundary_velocity_manager.get_prescribed_boundary_velocity_indicators().empty()
Additional information: 
    If we have inhomogeneous constraints, we must re-assemble the system
    matrix.

I haven't found the reason why the incompressible model works well while the compressible model doesn't. I'll try to figure it out.

[YiminJin]

I found that it is not a bug: it is because I should not use Newton method to solve this test problem. In this test, the viscosity is constant, and the only source of nonlinearity is density. In this case, the function Simulator::stokes_matrix_depends_on_solution() returns false, and will not rebuild the Stokes matrix in nonlinear iterations.

Therefore, I think none of the compressibility tests can be modified to test the Newton solver. We may have to build some tests with visco-plastic model (or dislocation creep model) and compressibility to test those assemblers.

[YiminJin]

I don't understand the test failure. It seems to be some missing (or extra) blank lines. I applied them, and this time it asks me to change back.

[gassmoeller]

Yes there is a technical problem with the github actions tester at the moment. Could you undo the test output changes with the empty lines? I think we can merge this afterwards.

[YiminJin]

I squashed the commits before the latest push, so I cannot restore the previous state easily. Therefore, I checked the change file and found the two modified screen output files (all other output files have 3 blank lines at the end, while those two have 5), and deleted the extra blank lines manually. I think it is identical to "undo" the changes.

[gassmoeller]

Ok, looks all good now, lets merge this.

Here it was simple to revert the changes manually, but if you get into such a situation in the future, here is what I would do:

git checkout main tests

will checkout only the files in the tests folder to exactly the state they have on the main branch. You would afterwards have to make a commit (or amend an existing commit), but it is a nice way to get the state of certain files to be exactly the state that is available on a different branch.