Open-Sn / opensn

open-source Sn software
https://open-sn.github.io/opensn/
Other
18 stars 16 forks source link

classic source iteration #22

Closed ragusa closed 6 days ago

ragusa commented 10 months ago

The traditional SI (classic Richardson) should be re-implemented in the code. It was removed a long time ago but it is a useful technique

Special attention should be paid to lines such as in modules/linear_boltzmann_solvers/lbs_solver/iterative_methods/lbs_iterative_methods.h

case IterativeMethod::CLASSICRICHARDSON:
case IterativeMethod::CLASSICRICHARDSON_CYCLES:
case IterativeMethod::KRYLOV_RICHARDSON:
case IterativeMethod::KRYLOV_RICHARDSON_CYCLES:
    return "richardson";

When krylov_richardson is used, an extra sweep is needed because it comes out of a KSP solve. For classic richardson, this will not be the case.

zhardy-lanl commented 10 months ago

Technically SI is still in the code, but it implemented in PETSc with Richardson iterations. I agree that it would be nice to see the actual source iteration algorithm in the code. This will spur on a discussion about how we want to ultimately structure LBS to support multiple solution techniques

wdhawkins commented 10 months ago

I don't mind using the PETSc Richardson implementation, but we need to add a convergence callback that does a point-wise convergence check instead of using the residual.

ragusa commented 10 months ago

Emerson asked Jan about it. I guess it is becoming urgent for his paper with Morel. Jan replied the following. This seems doable:

This is the file containing the convergence test. https://github.com/chi-tech/chi-tech/blob/development/modules/LinearBoltzmannSolvers/A_LBSSolver/IterativeMethods/wgs_convergence_test.cc

Its pretty simple, however, we get a huge benefit by getting the residual norm already computed.

For a point-wise test in here we would have a loop over cells, moments and groups.

This is what the old algorithm used to look like: https://github.com/chi-tech/chi-tech/blob/ChiTechv1.0.1/ChiModules/LinearBoltzmannSolver/IterativeOperations/lbs_compute_pwchange.cc

It really is a no-brainer. All we need to do is add the a call to ComputePointWiseChange and extend the input language to enable it

zhardy-lanl commented 10 months ago

I think we should have the PETSc Richardson and in house implementation at the end of the day. The prior should be used for production level calculations and the latter as a tool for new comers to get their feet wet.

When I first learned Chi-Tech, there was no PETSc Richardson iterations, and I was able to see each step of the source iteration scheme in the code. That helped me learn the architecture of the code more quickly by clicking through the different routines. If only the PETSc implementation were there, it would have been much more difficult to dive into the structure of the code because I would have had to figure out how the PETSc API worked to understand how the code was being executed.

wdhawkins commented 10 months ago

I think we should have the PETSc Richardson and in house implementation at the end of the day. The prior should be used for production level calculations and the latter as a tool for new comers to get their feet wet.

When I first learned Chi-Tech, there was no PETSc Richardson iterations, and I was able to see each step of the source iteration scheme in the code. That helped me learn the architecture of the code more quickly by clicking through the different routines. If only the PETSc implementation were there, it would have been much more difficult to dive into the structure of the code because I would have had to figure out how the PETSc API worked to understand how the code was being executed.

Good point. That's how I feel about PETSc now. 😆