[Core] response function interface #6594

armingeiser · 2020-03-24T11:36:18Z

This adds a top level interface for any kind of response functions. This allows to use them easily on python level e.g. for Optimization.

This interface was previously used in individual applications e.g. StructMech and ShapeOpt and would also be added to ConvectionDiffusion in #6584.

Suggested here: #6584 (comment)

armingeiser · 2020-03-24T11:37:12Z

kratos/python_scripts/response_functions/response_function_interface.py

-
-class ResponseFunctionBase(object):
-    """The base class for response functions. Each response function
+class ResponseFunctionInterface(object):


This is the new file in the core - Github shows it as moved...

Just out of curiosity: Is there a reason, why you called it "Interface" rather than "Base" or just "ResponseFunction"?

could you explain the relation btw this class and the AdjointResponseFunction?
This should help with the naming

The AdjointResponseFunction provides some of the necessary components for an adjoint sensitivity analysis:

gradient w.r.t to the primal variables (and their derivatives)

partial gradient w.r.t to the design variable

However it does not calculate the total gradient of the response function w.r.t the design variables.
This can only be obtained using the whole framework of adjoint_solver, adjoint_response_function and sensitivity_builder.

Besides the adjoint approach to calculate the gradient, there are several other ways - fore some e.g. geometrical response functions, the adjoint approach does not even make sense.

This response function interface provides a unique interface for all possible ways to calculate the value and gradients of a response.
This is necessary because e.g. in optimization i do not care about the implementation details (e.g. how the adjoint sensitivity analysis uses the adjoint_response_function.h ...) but i am just interested in the value and gradient of the response.

cool thx for the explanation

Could you please add sth like this to the docstring of the class?

MFusseder

I am fine with the suggested modifications. Especially I appreciate the more general get gradient functions. As written I would also add something like GetConditionalGradient.

kratos/python_scripts/response_functions/response_function_interface.py

dbaumgaertner

I would approve it as it looks good to me. Just some minor comments. It's good to see a unified interface of the response functions in the core. Thanks for the effort. Before merging though, it would be good to have somebody from the core guys to quickly double-check the PR @philbucher .

...ns/ShapeOptimizationApplication/python_scripts/response_functions/packaging_response_base.py

dbaumgaertner · 2020-03-25T09:17:47Z

kratos/python_scripts/response_functions/response_function_interface.py

-
-class ResponseFunctionBase(object):
-    """The base class for response functions. Each response function
+class ResponseFunctionInterface(object):


Just out of curiosity: Is there a reason, why you called it "Interface" rather than "Base" or just "ResponseFunction"?

kratos/python_scripts/response_functions/response_function_interface.py

dbaumgaertner · 2020-03-25T09:30:39Z

You might also remove the trailing whitespace as suggested by codacy

philbucher · 2020-03-25T10:04:26Z

I would like to review this but I need a few days, I would appreciate if you wait :)

armingeiser · 2020-03-25T10:29:18Z

I would like to review this but I need a few days, I would appreciate if you wait :)

No worries =)

philbucher

some comments

Just to be clear, this should also be used in Fluids?

philbucher · 2020-03-24T12:07:47Z

...ests/sensitivity_verification_process_test/run_sensitivity_verification_in_geometry_space.py

I would disable writing this in a test
It only takes time and is not used in 99% of times this test is run
and if you want to look at the output then you wouldn't delete it ;)

...ns/ShapeOptimizationApplication/python_scripts/response_functions/packaging_response_base.py

kratos/python_scripts/response_functions/response_function_interface.py

philbucher · 2020-03-31T10:58:20Z

kratos/python_scripts/response_functions/response_function_interface.py

-
-class ResponseFunctionBase(object):
-    """The base class for response functions. Each response function
+class ResponseFunctionInterface(object):


could you explain the relation btw this class and the AdjointResponseFunction?
This should help with the naming

philbucher · 2020-03-31T11:00:26Z

...tion/tests/sensitivity_verification_process_test/run_semi_analytic_step_size_verification.py

I see the point, however this would be something for a new MR, as this is just a whitespace change here and not related.

philbucher · 2020-03-31T11:03:14Z

Idk why but my comments are inline with the comments of @dbaumgaertner
=> go to Files changed then it is more readable

armingeiser · 2020-03-31T11:36:33Z

Just to be clear, this should also be used in Fluids?
@philbucher

It can, but i have no plans currently. The Fluid/Structural/ConvDiff use the same workflow and base classes for their adjoint sensitivity analysis - so it should be possible.

One point that is worth mentioning is that in for structures there is sensitivity analysis only for steady state sensitivities, while for fluid it is only (?) available for transient cases.

armingeiser · 2020-04-02T13:59:45Z

@marcnunezc This is essentially the interface i gave you some time ago to test optimization with the CompressiblePotentialFlowApplication.

marcnunezc

@marcnunezc This is essentially the interface i gave you some time ago to test optimization with the CompressiblePotentialFlowApplication.

Yes! I have been using that as a custom script ever since, thanks for the effort!

I am OK with the general response interface, I only have a question regarding the reporting of the gradient.

applications/ShapeOptimizationApplication/python_scripts/analyzer_internal.py

philbucher · 2020-04-03T08:01:21Z

so from my side this is ready to go in if the involved ppl are ok with it
@KratosMultiphysics/technical-committee I think we don't need to review this in detail for the same reason (we have enough more important things to do)
@marcnunezc I suggest to approve if you are ok with it

thanks for the efforts!

philbucher · 2020-04-03T08:04:15Z

@armingeiser I think you didn't set your editor to auto-delete trailing whitespaces

This happens when editing files using the github online file editor =)

marcnunezc

Ok!

armingeiser added 3 commits March 24, 2020 11:53

add interface to core

86f08cc

use core interface in existing code

bac23f7

minor fix

af8afc8

armingeiser requested review from dbaumgaertner and MFusseder March 24, 2020 11:36

armingeiser commented Mar 24, 2020

View reviewed changes

MFusseder reviewed Mar 24, 2020

View reviewed changes

kratos/python_scripts/response_functions/response_function_interface.py Show resolved Hide resolved

armingeiser added 2 commits March 24, 2020 17:17

add get gradient function for conditions

8c1918e

codacy

3610eb2

dbaumgaertner previously approved these changes Mar 25, 2020

View reviewed changes

armingeiser requested review from philbucher and rubenzorrilla March 25, 2020 10:29

philbucher added this to To Do in TODO philbucher via automation Mar 26, 2020

philbucher moved this from To Do to To be reviewed in TODO philbucher Mar 26, 2020

armingeiser added 2 commits March 26, 2020 19:37

Merge branch 'master' into Core/response-function-interface

081d6b1

use core response interface

845015a

armingeiser dismissed dbaumgaertner’s stale review via 845015a March 26, 2020 18:41

philbucher reviewed Mar 31, 2020

View reviewed changes

philbucher requested review from sunethwarna and marcnunezc March 31, 2020 17:44

update docstring of interface class

89ccdf9

armingeiser marked this pull request as ready for review April 2, 2020 13:57

armingeiser requested a review from a team as a code owner April 2, 2020 13:57

marcnunezc reviewed Apr 2, 2020

View reviewed changes

applications/ShapeOptimizationApplication/python_scripts/analyzer_internal.py Show resolved Hide resolved

remove trailing whitespaces

4fb0a1e

marcnunezc approved these changes Apr 3, 2020

View reviewed changes

armingeiser merged commit 349c02b into master Apr 6, 2020

TODO philbucher automation moved this from To be reviewed to Done Apr 6, 2020

armingeiser deleted the Core/response-function-interface branch April 6, 2020 13:40

armingeiser added a commit that referenced this pull request Apr 8, 2020

fix wrong import after #6594

4f7013b

armingeiser added a commit that referenced this pull request Apr 8, 2020

fix wrong import after #6594 (#6735)

8525443

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] response function interface #6594

[Core] response function interface #6594

armingeiser commented Mar 24, 2020

armingeiser Mar 24, 2020

dbaumgaertner Mar 25, 2020

philbucher Mar 31, 2020

armingeiser Mar 31, 2020

philbucher Mar 31, 2020

MFusseder left a comment

dbaumgaertner left a comment

dbaumgaertner Mar 25, 2020

dbaumgaertner commented Mar 25, 2020

philbucher commented Mar 25, 2020

armingeiser commented Mar 25, 2020

philbucher left a comment

philbucher Mar 24, 2020

philbucher Mar 31, 2020

philbucher Mar 31, 2020

armingeiser Mar 31, 2020

philbucher commented Mar 31, 2020

armingeiser commented Mar 31, 2020

armingeiser commented Apr 2, 2020

marcnunezc left a comment

philbucher commented Apr 3, 2020

philbucher commented Apr 3, 2020 •

edited by armingeiser

marcnunezc left a comment

[Core] response function interface #6594

[Core] response function interface #6594

Conversation

armingeiser commented Mar 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MFusseder left a comment

Choose a reason for hiding this comment

dbaumgaertner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbaumgaertner commented Mar 25, 2020

philbucher commented Mar 25, 2020

armingeiser commented Mar 25, 2020

philbucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philbucher commented Mar 31, 2020

armingeiser commented Mar 31, 2020

armingeiser commented Apr 2, 2020

marcnunezc left a comment

Choose a reason for hiding this comment

philbucher commented Apr 3, 2020

philbucher commented Apr 3, 2020 • edited by armingeiser

marcnunezc left a comment

Choose a reason for hiding this comment

philbucher commented Apr 3, 2020 •

edited by armingeiser