Discussion: Remove the inheritance of Epetra_Operator from SparseOperator?

[knarfnitram]

Description and Context

The SparseOperator which is the the general virtual Interface for the 4c matrices, inherits directly from the Epetra_Operator, offering to access functions from Epetra directly within our codebase.
I tried to remove the inheritance in a first step, but came up with some issues related to nox.

Since the inheritance is removed, the still necessary Epetra_Operator could be reused from the stored matrix as done with the SparseMatrix or for our custom implementations (BlockSparseMatrixBase), where we overwrite methods of the Epetra_Operator we would still need to directly inherit from the Epetra_Operator (according to @maxfirmbach we could probably also remove this inheritance by switching from the belos interface to thyra for the iterative solver interface).

This PR is meant as discussion with some code example.
Open questions:

• How can this be aligned with our new structure time integration (eg. nox) since the Epetra_Operator is used there.
  How is it planed to be used?
• What do you think about the general design of this interface?
• I'd be happy to hear your opinions @maxfirmbach @vovannikov @sebproell

Related Issues and Pull Requests

[maxfirmbach]

I think this is a good first approach due to several reasons:

1. It allows us to define and steer our own abstract matrix interface.
2. It isolates the inheritance to a place, where it is actually used. Our SparseMatrix holds an Epetra_CrsMatrix inside its own wrapper, so no need to inherit from Epetra_Operator.

[sebproell]

Suggested change

/*LINALG_SPARSEOPERATOR_H_*/

[sebproell]

Suggested change

	virtual ~SparseOperator(); // declaration only
	virtual ~SparseOperator() = default;

and remove the corresponding cpp file.

[sebproell]

This is correct AFAICT, however, aliasing shared_ptrs is complicated enough that it might warrant its own helper function.

[maxfirmbach]

@vovannikov What's your take on the nonlinear solver? From the linear solver perspective I'm pretty confident, that we can get rid of the inheritance by using the Thyra based interface and conversion utilities.

[vovannikov]

@knarfnitram Thank you for this effort!

In my opinion, Epetra_Operator should get wiped out completely from 4C. I think creating our own wrappers is the way to go. It is fine that some of these wrappers may be inherited at the moment, but eventually this inheritance gets broken, once the code is ready for that.

Similarly here, this inheritance from Epetra_Operator should be broken, but I have a feeling that it is a bit too early to do this, as it requires a lot of work within a single PR. One may be prone to adhering to fast but suboptimal solutions here and there in the code, since at the current state, breaking this inheritance is indeed challenging: the code is not yet ready for this.

It is possible to make NOX independent from Epetra since, NOX, by design, contains generalized abstract base classes. The classes from NOX::Epetra:: should not be used eventually. This can not be achieved immediately, but I have been steadily working on this. I have recently also added our own implementation of the abstract NOX vector (#1399, #1445). Similar idea should be used for other NOX related components.

@maxfirmbach I think you are right. Thyra + the conversion utils is the way to go, I have seen how you made it for preconditioners, nice. I have a similar plan for the NOX refactoring, however, I would also like to wrap Thyra operators inside our own wrapper, or something like that.

[knarfnitram]

Thanks for the update @vovannikov. Good to hear, that we are on the same page. Just let us know if you need any help regarding the matrix interfaces or once the NOX interface is ready such that we can continue here.