bifurcationkit / BifurcationKit.jl

A Julia package to perform Bifurcation Analysis
https://bifurcationkit.github.io/BifurcationKitDocs.jl/stable
Other
288 stars 34 forks source link

Custom BorderedLinearSolver interface requirements for PALC are missing #140

Open Azercoco opened 4 months ago

Azercoco commented 4 months ago

When using a custom AbstractBorderedLinearSolver as the bls field in PALC( ...), the required interface methods as mentioned here are not sufficient. The custom bordered solver is also expected to implements something like this .

It also seems that this implementation is exactly the same for all sub types of AbstractBorderedLinearSolver. Maybe it would be better to just implements this method for the AbstractBorderedLinearSolver type directly instead of having duplicated code.

Azercoco commented 4 months ago

Also in PALC, the solver is expected to have a field solver, which can be equal to nothing but has to be present nevertheless or the compilation will fail.

rveltz commented 4 months ago

Hi

Thank you for reporting this.

Maybe it would be better to just implements this method for the AbstractBorderedLinearSolver type directly instead of having duplicated code.

I agree, thank you for the suggestion.

rveltz commented 4 months ago

At some point, I need to find a better way for specifying the interfaces. As for PALC, I guess you are referring to the update method. I guess I can correct this by adding the requirement that an update method exists for AbstractBorderedLinearSolver.

Is this blocking you or you managed to find a way out?

Azercoco commented 4 months ago

Yes, that was the issue, the work around I found was just to add an solver field with a nothing value. No the cleanest but it did work.

rveltz commented 1 month ago

You are right for the required field solver.

When using a custom AbstractBorderedLinearSolver as the bls field in PALC( ...), the required interface methods as mentioned here are not sufficient. The custom bordered solver is also expected to implements something like this .

This I am not sure. It is a fallback that will use the custom bordered linear solver, no need to add this.