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

[parameters/functionals] Add derivative of products #934

Merged
merged 12 commits into from Jun 18, 2020

Conversation

TiKeil
Copy link
Contributor

@TiKeil TiKeil commented May 29, 2020

If the ProductParameterFunctional is simply a number times a ParameterFunctional, we can easily compute the d_mu for this. However, it is required to automatically detect this case. So, we first count the actual number of ParameterFunctionals in the product and compute the derivative in case, there is only one ParameterFunctional .
With this code, one can then go ahead and implement product rules and generalizations for more parameters, if needed.

@codecov
Copy link

@codecov codecov bot commented May 29, 2020

Codecov Report

Merging #934 into master will increase coverage by 0.02%.
The diff coverage is 91.66%.

Impacted Files Coverage Δ
src/pymor/parameters/functionals.py 93.68% <91.66%> (+0.39%) ⬆️

@sdrave
Copy link
Member

@sdrave sdrave commented Jun 2, 2020

@TiKeil, is there a reason for not just implementing the product rule and checking the individual summands for being zero?

@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Jun 2, 2020

You are right, this would probably be the most rigorous way of implementing it. The reason why I did not do it was that for my applications I only needed this very simple special case.

@TiKeil TiKeil force-pushed the add_derivative_of_simple_products branch from da79228 to d19857c Compare Jun 3, 2020
@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Jun 3, 2020

@sdrave , I think I have implemented what you suggested in d19857c. This code still only works if there is only one factor which has a non vanishing derivative.

I think we can not support derivatives of products with more than one summand in the product rule unless we implement the '+' operator for ParameterFunctionals. What about providing it?

@sdrave
Copy link
Member

@sdrave sdrave commented Jun 8, 2020

I think we can not support derivatives of products with more than one summand in the product rule unless we implement the '+' operator for ParameterFunctionals. What about providing it?

Yes, I think we should add a LincombParameterFunctional. Do you want to do it or shall I?

Would you like to see this PR in the next release or is it ok to wait after 2020.1?

@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Jun 8, 2020

I think we can not support derivatives of products with more than one summand in the product rule unless we implement the '+' operator for ParameterFunctionals. What about providing it?

Yes, I think we should add a LincombParameterFunctional. Do you want to do it or shall I?

I can add it. In a separate PR I guess?

Would you like to see this PR in the next release or is it ok to wait after 2020.1?

I think after adding LincombParameterFunctional, this PR should not be to much work anymore. So in principle I would like to add it to 2020.1.

@sdrave sdrave added this to the 2020.1 milestone Jun 9, 2020
@sdrave sdrave added generic-objects pr:new-feature labels Jun 9, 2020
@sdrave
Copy link
Member

@sdrave sdrave commented Jun 9, 2020

Yes, I think we should add a LincombParameterFunctional. Do you want to do it or shall I?
I can add it. In a separate PR I guess?

Great! Yes.

I think after adding LincombParameterFunctional, this PR should not be to much work anymore. So in principle I would like to add it to 2020.1.

Fine, I added it to the milestone.

@TiKeil TiKeil force-pushed the add_derivative_of_simple_products branch from d19857c to 42ef5fc Compare Jun 17, 2020
@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Jun 17, 2020

I used #950 to finish the product rule and added a more advanced test for it

@TiKeil TiKeil changed the title [parameters/functionals] Add derivative of simple products [parameters/functionals] Add derivative of products Jun 17, 2020
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
src/pymor/parameters/functionals.py Outdated Show resolved Hide resolved
TiKeil and others added 4 commits Jun 18, 2020
Co-authored-by: Stephan Rave <stephanrave@uni-muenster.de>
Co-authored-by: Stephan Rave <stephanrave@uni-muenster.de>
@TiKeil TiKeil force-pushed the add_derivative_of_simple_products branch from c31998e to 42d693b Compare Jun 18, 2020
@TiKeil
Copy link
Contributor Author

@TiKeil TiKeil commented Jun 18, 2020

This seems to be done

@sdrave sdrave merged commit 8a6e4cd into pymor:master Jun 18, 2020
8 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

2 participants