Simplify dolfin.Form, assemble, and assemble_system by removing arguments to override ufl.Form contents

[Martin Sandve Alnæs]

This would make the code much cleaner and simplify a lot.

The question is, does anyone use the overrides, or which of them are useful?

We have:

class Form(cpp.Form):
    def __init__(self, form,
                 function_spaces=None,
                 coefficients=None,
                 subdomains=None,
                 form_compiler_parameters=None,
                 common_cell=None, # deprecated
                 mesh=None):

There are three main scenarios:

1. form can be a ufl.Form. This is the main usage scenario.
2. form can be a cpp.Form. Useful?
3. form can be a ufc.form. Useful?

Next we have:

• form_compiler_parameters is still useful.
• common_cell is deprecated and will be removed.

Which leaves:

1. function_spaces to override the function spaces of the test/trial functions. Useful?
2. coefficients can override the coefficient list from form. Useful?
3. subdomains can override the meshfunctions from form. Useful?
4. mesh can override the integration mesh. Useful?

One simplification would be to say (function_space, coefficients, subdomains, mesh) is only allowed if form is a ufc.form. Then there are three code paths:

1. get data from ufl.Form if that's what we have
2. get data from cpp.Form if we have a copy constructor
3. get data from arguments if we have a ufc.form

[Martin Sandve Alnæs]

Btw this is not just for prettyness, this would actually make my life easier in some future updates.

[Prof Garth Wells]

I'm all for simplifying things and not have two+ ways to so something without good reason. I would remove support for the four points under 'Which leaves:', and I support the proposed simplification at the end of the post.

[Martin Sandve Alnæs]

Good. I'll add deprecation warnings now and apply the changes after 1.4.

[Martin Sandve Alnæs]

@johanhake and I discussed this a bit and concluded that to streamline the ufl/dolfin interface we should remove all the arguments to Form and assemble* that provide data that should be part of the ufl form. I will add a bunch of deprecation warnings to Form and assemble* now and simplify heavily after the 1.4 release.

[Martin Sandve Alnæs]

The new signatures will then be:

class Form(cpp.Form):
    def __init__(self, form, form_compiler_parameters=None):

def assemble(form,
             form_compiler_parameters=None,

             # Removed after 1.4:
             #mesh=None,
             #coefficients=None,
             #function_spaces=None,

             # Removed after 1.4 or 1.5?:
             #cell_domains=None,
             #exterior_facet_domains=None,
             #interior_facet_domains=None,

             tensor=None,
             reset_sparsity=None,
             add_values=False,
             finalize_tensor=True,
             keep_diagonal=False,
             backend=None,
             bcs=None):

def assemble_system(A_form,
                    b_form,
                    bcs=None,
                    x0=None,

                    # Removed after 1.4:
                    #A_coefficients=None,
                    #b_coefficients=None,
                    #A_function_spaces=None,
                    #b_function_spaces=None,
                    #mesh=None,

                    # Removed after 1.4 or 1.5?:
                    #cell_domains=None,
                    #exterior_facet_domains=None,
                    #interior_facet_domains=None,

                    reset_sparsity=None,
                    add_values=False,
                    finalize_tensor=True,
                    keep_diagonal=False,
                    A_tensor=None,
                    b_tensor=None,
                    backend=None,
                    form_compiler_parameters=None
                    ):


class SystemAssembler(cpp.SystemAssembler):
    __doc__ = cpp.SystemAssembler.__doc__
    def __init__(self, A_form, b_form,
                 bcs=None,
                 form_compiler_parameters=None,

                 # Removed after 1.4:
                 #A_coefficients=None,
                 #b_coefficients=None,
                 #A_function_spaces=None,
                 #b_function_spaces=None,
                 #mesh=None,

                 # Removed after 1.4 or 1.5?:
                 #cell_domains=None,
                 #exterior_facet_domains=None,
                 #interior_facet_domains=None,
                 ):

One question I have still: is it ok to deprecate assemble(..., exterior_facet_domains=...) and delete it already after the 1.4 release or should we keep that until 1.5?

[Prof Garth Wells]

reset_sparsity=None can be removed. Resetting tensor sparsity has been deprecated in the backends, i.e. the option has not effect in the dev version and the ability to set the 'option' will be removed soon.

[Martin Sandve Alnæs]

reset_sparsity is already marked with deprecation for after this release.

[Martin Sandve Alnæs]

Actually, the combination of passing a dolfin cpp.Form to assemblers and passing exterior_facet_domains etc. is dangerous and buggy, because the cpp.Form object will be modified behind the scenes. I don't see a good way to fix that bug, so I vote for deprecating the *_domains arguments to python assemblers and removing already after 1.4.

[Martin Sandve Alnæs]

Ok, here's my solution for now: I'm adding warnings when assemble and friends do fishy things like modifying the input dolfin form. And I'm deprecating *_domains with scheduled removal in 1.6, because they're used a lot so people will need some time to adjust.

[Martin Sandve Alnæs]

Deprecations and warnings in dolfin next now.

[Martin Sandve Alnæs]

Further possible simplifications:

1) Do we need the 'backend'? If None, the fallback is to use cpp.Vector(), which results in using the backend chosen by the global parameter "linera_algebra_backend". Isn't that sufficient?

2) Can we group { add_values, finalize_tensor, keep_diagonal } into an assembler_parameters dict? That will further shorten the still quite long argument list and simplify future changes in assembler parameters.

At least (1) can be added now as a deprecated argument and easily removed later.

[Prof Garth Wells]

1) I say remove.

2) Yes, we could group these together.

[Martin Sandve Alnæs]

However, it may have a rare use case and the code is not in the way, so I won't bother.

[Martin Sandve Alnæs]

Deprecations and warnings in dolfin master now, 18683f4c6081e4125e5c5b2aaddd0c33ee88111e

[Martin Sandve Alnæs]

Work in next now.

[Martin Sandve Alnæs]

5b905dfc4ccf24fb86575f5bf6121fe65631e074

[Prof Garth Wells]

Removing milestone: 1.5 (automated comment)