Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add partial derivatives w.r.t. parameters #748

Merged
merged 13 commits into from Aug 28, 2019

Conversation

TiKeil
Copy link
Contributor

@TiKeil TiKeil commented Aug 14, 2019

This pull request adds a basic version of parameters derivatives to pymor.

  1. Parameters package:

    • ParameterFunctionalInterface has now a partial_derivative method that returns a new ParameterFunctional representing the derivative to a specific component and coordinate of the parameter

    • For this pull request, it is only possible to differentiate ProjectionParameterFunctional and ExpressionParameterFunctional. For the latter the user has to provide a dict that contains all partial derivatives of the given Parameter. An assertion on the size of this dict might be useful. All shapes of parameters are possible ( scalar, vector and matrix)

  2. Operators package:

    • I added a mu_derivative method to the OperatorInterface and thus also to OperatorBase. This method returns a new Operator representing the old Operator differentiated to a certain component and coordinate of the parameter.

    • For this pull request, I only implement the mu_derivative to LincombOperator. This function returns a new LincombOperator with the differentiated ParameterFunctional. Note that this only makes sense mathematically if the operators of the LincombOperator are not parametric. Product rules are not implmented, yet.

  3. The names of the methods and members are just in a first-choice fashion. I am very open for other suggestions

  4. I added a small test to the pymortests where I show the usage of the new functions. I did not use pytest or something else yet. Is that required?

  5. There are multiple things to do that I left for future work for now:

    a) implement a numeric version of the partial derivative of the parameters
    b) Use sympy to get automatic expressions for the derivative.
    c) Add partial derivative for ProductParameterFunctional and ConjugateParameterFunctional
    d) Add mu_derivative for more Operators

Comments and tips are very welcome

Copy link
Member

@renefritze renefritze left a comment

The different sections here could be better reflected as test functions. This would make error reporting through pytest follow your intent of issue separation here. Those test functions are also easy to evolve into accepting parameterized input from data generating fixtures.

Edit: Oops, this should ofc be a comment on the new file under src/pymortests/

@codecov
Copy link

@codecov codecov bot commented Aug 14, 2019

Codecov Report

Merging #748 into master will increase coverage by 0.02%.
The diff coverage is 80.55%.

Impacted Files Coverage Δ
src/pymor/operators/basic.py 62.77% <25%> (-0.86%) ⬇️
src/pymor/operators/interfaces.py 69.23% <66.66%> (-0.13%) ⬇️
src/pymor/parameters/interfaces.py 79.16% <66.66%> (-1.79%) ⬇️
src/pymor/parameters/functionals.py 91.83% <84.31%> (-8.17%) ⬇️
src/pymor/operators/constructions.py 85.42% <90.9%> (+0.1%) ⬆️

@codecov
Copy link

@codecov codecov bot commented Aug 14, 2019

Codecov Report

Merging #748 into master will increase coverage by 0.05%.
The diff coverage is 84.21%.

Impacted Files Coverage Δ
src/pymor/operators/basic.py 62.77% <25%> (-0.86%) ⬇️
src/pymor/operators/interfaces.py 69.23% <66.66%> (-0.13%) ⬇️
src/pymor/parameters/functionals.py 93.61% <86.36%> (-6.39%) ⬇️
src/pymor/operators/constructions.py 85.39% <90%> (+0.08%) ⬆️
src/pymor/parameters/interfaces.py 86.11% <93.33%> (+5.15%) ⬆️

@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Aug 14, 2019

I updated the tests following the comment from @renefritze

Copy link
Member

@sdrave sdrave left a comment

Apart from some minor issues, this is looking very nice already! I would say this can be merged after resolving these issues.

src/pymor/operators/interfaces.py Outdated Show resolved Hide resolved
src/pymor/operators/interfaces.py Outdated Show resolved Hide resolved
src/pymor/operators/basic.py Outdated Show resolved Hide resolved
src/pymor/operators/constructions.py Outdated Show resolved Hide resolved
src/pymor/operators/constructions.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymortests/mu_derivatives.py Outdated Show resolved Hide resolved
@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Aug 26, 2019

I pushed changes proposed by @sdrave .

I also added ConstantParameterFunctional. It only takes a constant_value and returns this value no matter what the parameter is. Furthermore I allow all types of constant values. Moreover this functional has no parameter_type and thus the user can evaluate this functional with every mu. Any objections for that?

Otherwise, this PR can be merged.

Edit: I rebased on master

src/pymor/operators/basic.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Aug 27, 2019

I have pushed new changes according to the requests of @sdrave.
I added a function _check_and_parse_input in order to be able to check the input of d_mu for every ParameterFunctional. This can be used for every ParameterFunctional now.

Copy link
Member

@sdrave sdrave left a comment

You should also add yourself to AUTHORS.md.

src/pymor/operators/interfaces.py Outdated Show resolved Hide resolved
src/pymor/operators/interfaces.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/interfaces.py Outdated Show resolved Hide resolved
src/pymor/parameters/interfaces.py Outdated Show resolved Hide resolved
@sdrave
Copy link
Member

@sdrave sdrave commented Aug 27, 2019

Moreover this functional has no parameter_type and thus the user can evaluate this functional with every mu. Any objections for that?

No objections.

@TiKeil TiKeil force-pushed the parameter_derivatives branch from a6e9916 to 59e24d8 Compare Aug 28, 2019
@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Aug 28, 2019

I think this can be merged in case there are no other objections.

@sdrave sdrave added this to the 2019.2 milestone Aug 28, 2019
@sdrave sdrave added interfaces pr:new-feature labels Aug 28, 2019
sdrave
sdrave approved these changes Aug 28, 2019
@sdrave sdrave changed the title Add partial derivatives of parameters Add partial derivatives w.r.t. parameters Aug 28, 2019
@sdrave sdrave merged commit 58add64 into pymor:master Aug 28, 2019
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:new-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants