Kratos: [Core] residualbased linear strategy + reform_dofs_at_each_step = 0 norm always

[adityaghantasala]

Currently the solve function of the ResidualBasedLinearStrategy has the following Solve() function

   double Solve() override
    {
        BaseType::Solve();

        //calculate if needed the norm of Dx
        double norm_dx = 0.00;
        if (mCalculateNormDxFlag == true)
            norm_dx = TSparseSpace::TwoNorm(*mpDx);

        return norm_dx;
    }

This when used together with the flag reform_dofs_at_each_step=true will always return a zero value for norm_dx. This is because the Clear() function of the strategy which will be called in FinalizeSolutionStep() will clear mpDx thus emptying it. And so always a zero norm is returned.

To avoid this, suggestion is to change the above function as follows

    double Solve() override
    {
        Initialize();
        InitializeSolutionStep();
        Predict();
        SolveSolutionStep();

        //calculate if needed the norm of Dx
        double norm_dx = 0.00;
        if (mCalculateNormDxFlag == true)
            norm_dx = TSparseSpace::TwoNorm(*mpDx);

        FinalizeSolutionStep();
        return norm_dx;
    }

[RiccardoRossi]

actually we should Remove the function Solve, and call instead the
different parts,

other than that i think that your change is correct.

On Thu, Apr 9, 2020 at 3:57 AM Aditya Ghantasala [e-mail]
wrote:

Currently the solve function of the ResidualBasedLinearStrategy has the
following Solve() function

double Solve() override
{
BaseType::Solve();

    //calculate if needed the norm of Dx
    double norm_dx = 0.00;
    if (mCalculateNormDxFlag == true)
        norm_dx = TSparseSpace::TwoNorm(*mpDx);

    return norm_dx;
}

This when used together with the flag reform_dofs_at_each_step=true will
always return a zero value for norm_dx. This is because the Clear()
function of the strategy which will be called in FinalizeSolutionStep()
will clear mpDx thus emptying it. And so always a zero norm is returned.

To avoid this, suggestion is to change the above function as follows

double Solve() override
{
    Initialize();
    InitializeSolutionStep();
    Predict();
    SolveSolutionStep();

    //calculate if needed the norm of Dx
    double norm_dx = 0.00;
    if (mCalculateNormDxFlag == true)
        norm_dx = TSparseSpace::TwoNorm(*mpDx);

    FinalizeSolutionStep();
    return norm_dx;
}

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/KratosMultiphysics/Kratos/issues/6748, or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB5PWELB2LKDAOGFPHRI6E3RLUTPBANCNFSM4MELZ7GA
.

--

Riccardo Rossi

PhD, Civil Engineer

member of the Kratos Team: www.cimne.com/kratos

Associate Professor at Universitat Politècnica de Catalunya, BarcelonaTech
(UPC)

Full Research Professor at International Center for Numerical Methods in
Engineering (CIMNE)

C/ Gran Capità, s/n, Campus Nord UPC, Building C1, First Floor

08034 – Barcelona – Spain – www.cimne.com -

T.([phone] skype: rougered4

http://www.cimne.com/

https://www.facebook.com/cimne http://blog.cimne.com/
http://vimeo.com/cimne http://www.youtube.com/user/CIMNEvideos
http://www.linkedin.com/company/cimne https://twitter.com/cimne

Les dades personals contingudes en aquest missatge són tractades amb la
finalitat de mantenir el contacte professional entre CIMNE i voste. Podra
exercir els drets d'accés, rectificació, cancel·lació i oposició,
dirigint-se a [email protected]. La utilització de la seva adreça de
correu electronic per part de CIMNE queda subjecte a les disposicions de la
Llei 34/2002, de Serveis de la Societat de la Informació i el Comerç
Electronic.

Imprimiu aquest missatge, només si és estrictament necessari.
http://www.cimne.com/

[loumalouomega]

actually we should Remove the function Solve, and call instead the different parts, other than that i think that your change is correct.

do you mean in the python scripts?, I assume Solve should still exist (it is still used many places)

[loumalouomega]

Currently the solve function of the ResidualBasedLinearStrategy has the following Solve() function

   double Solve() override
    {
        BaseType::Solve();

        //calculate if needed the norm of Dx
        double norm_dx = 0.00;
        if (mCalculateNormDxFlag == true)
            norm_dx = TSparseSpace::TwoNorm(*mpDx);

        return norm_dx;
    }

This when used together with the flag reform_dofs_at_each_step=true will always return a zero value for norm_dx. This is because the Clear() function of the strategy which will be called in FinalizeSolutionStep() will clear mpDx thus emptying it. And so always a zero norm is returned.

To avoid this, suggestion is to change the above function as follows

    double Solve() override
    {
        Initialize();
        InitializeSolutionStep();
        Predict();
        SolveSolutionStep();

        //calculate if needed the norm of Dx
        double norm_dx = 0.00;
        if (mCalculateNormDxFlag == true)
            norm_dx = TSparseSpace::TwoNorm(*mpDx);

        FinalizeSolutionStep();
        return norm_dx;
    }

First of all, maybe we should define what is supposed to be returned from the Solve (because right now it is only done in the LinearStrategy)

[loumalouomega]

actually we should Remove the function Solve, and call instead the different parts, other than that i think that your change is correct.

do you mean in the python scripts?, I assume Solve should still exist (it is still used many places)

Or you mean that instead of calling the base class solve to call to call each method as @adityaghantasala showed?

[RiccardoRossi]

I tend to think that we should never use Solve(). That is already split in
the AnalysisStage and that's how it should be

Riccardo

On Thu, Apr 9, 2020 at 10:25 AM Vicente Mataix Ferrándiz <
[email protected]> wrote:

actually we should Remove the function Solve, and call instead the
different parts, other than that i think that your change is correct.

do you mean in the python scripts?, I assume Solve should still exist (it
is still used many places)

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/KratosMultiphysics/Kratos/issues/6748#issuecomment-611401188,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB5PWEOXOSGJZLZKB4J3GFDRLWBBPANCNFSM4MELZ7GA
.

--

Riccardo Rossi

PhD, Civil Engineer

member of the Kratos Team: www.cimne.com/kratos

Associate Professor at Universitat Politècnica de Catalunya, BarcelonaTech
(UPC)

Full Research Professor at International Center for Numerical Methods in
Engineering (CIMNE)

C/ Gran Capità, s/n, Campus Nord UPC, Building C1, First Floor

08034 – Barcelona – Spain – www.cimne.com -

T.([phone] skype: rougered4

http://www.cimne.com/

https://www.facebook.com/cimne http://blog.cimne.com/
http://vimeo.com/cimne http://www.youtube.com/user/CIMNEvideos
http://www.linkedin.com/company/cimne https://twitter.com/cimne

Les dades personals contingudes en aquest missatge són tractades amb la
finalitat de mantenir el contacte professional entre CIMNE i voste. Podra
exercir els drets d'accés, rectificació, cancel·lació i oposició,
dirigint-se a [email protected]. La utilització de la seva adreça de
correu electronic per part de CIMNE queda subjecte a les disposicions de la
Llei 34/2002, de Serveis de la Societat de la Informació i el Comerç
Electronic.

Imprimiu aquest missatge, només si és estrictament necessari.
http://www.cimne.com/

[adityaghantasala]

One usage of the Solve() function is here

https://github.com/KratosMultiphysics/Kratos/blob/07a2f60d6e0f375fcd413230ec77e871ba55aed0/applications/FluidDynamicsApplication/custom_strategies/strategies/fs_strategy.h#L574

Then according to your suggestion @RiccardoRossi this one line would change as follows

mpMomentumStrategy->Initialize();
mpMomentumStrategy->InitializeSolutionStep();
mpMomentumStrategy->Predict();
mpMomentumStrategy->SolveSolutionStep();
norm_dx = TSparseSpace::TwoNorm(mpMomentumStrategy->GetSolutionVector());

[RiccardoRossi]

yes.

Having said this
1 - i am not against your change
2 - also the solve of the FS strategy should be eventually removed and split in its components ... (even if not as a part of this PR)

mentioning @rubenzorrilla

[adityaghantasala]

First of all, maybe we should define what is supposed to be returned from the Solve (because right now it is only done in the LinearStrategy)

@loumalouomega If we decide to keep the Solve() function, I think it should always return the residual norm. Not like what is done in linear_strategy.

[philbucher]

Well, for just replacing it its ok

But then you are calling Initialize etc in SolveSolutionStep which is bad (I understand this is the current behavior)

=> In the future this should be cleaned properly

[philbucher]

In a nutshell, ++++++1 for removing Solve for good, this has given us too many issues in the past

[rubenzorrilla]

2 - also the solve of the FS strategy should be eventually removed and split in its components ... (even if not as a part of this PR)

Added to the Fluid Dynamics Application project.

[adityaghantasala]

2 - also the solve of the FS strategy should be eventually removed and split in its components ... (even if not as a part of this PR)

@rubenzorrilla I can make the PR if you like, I am already at those changes for ChimeraApp

[loumalouomega]

In a nutshell, ++++++1 for removing Solve for good, this has given us too many issues in the past

I am not in favor of removing Solve, it is used in many Unnitest, and it removal will increase the verbosity. I prefer to manually change it's call in everyplace we can and we should

[philbucher]

you are right, it is used often

For me would be ok to leave it in Python for now, but the C++ version IMO should be deprecated

I remember that I cleaned it up in many places long time ago, but in some places it is still used

[adityaghantasala]

@RiccardoRossi the suggested changes above will also require that we add GetSystemMatrix , GetSystemVector , GetSolutionVector methods to the base SolvingStrategy. Is this Ok ?

Is there a specific reason if these methods are not in base class from the beginning ?

[pooyan-dadvand]

@KratosMultiphysics/technical-committee respect to this issue and the relative problems decides to remove the Solve method of the solver in favor of its part by part methods as a clean solution.

@KratosMultiphysics/all This method should be deprecated in in a separate PR and eventually removed from the code.