ROL: changes to Thyra Adapters for SimOpt interface #7416
Conversation
mperego
added
type: enhancement
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_9.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
Using Repos:
Pull Request Author: mperego |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_9.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_4.8.4 # 6786 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_intel_17.0.1 # 6598 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_gcc_4.9.3_SERIAL # 5023 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0 # 4870 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_gcc_8.3.0 # 1060 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_cuda_9.2 # 4360 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_clang_9.0.0 # 735 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_python_2 # 2497 (click to expand)Console Output (last 100 lines) : Trilinos_pullrequest_python_3 # 2508 (click to expand) |
- improved performance by carefully avoiding recomputing objects - output messages useful for debug purposes - added ASSERTS in debug mode to ensure parameters are properly updated
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_9.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
Using Repos:
Pull Request Author: mperego |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_9.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_2
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
|
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
3 similar comments
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
| ThyraProductME_Constraint_SimOpt(Thyra::ModelEvaluatorDefaultBase<double>& thyra_model_, int g_index_, const std::vector<int>& p_indices_,Teuchos::RCP<Teuchos::ParameterList> params_ = Teuchos::null, bool recompute = false) : | ||
| thyra_model(thyra_model_), g_index(g_index_), p_indices(p_indices_), params(params_) { | ||
| ThyraProductME_Constraint_SimOpt(Thyra::ModelEvaluatorDefaultBase<double>& thyra_model_, int g_index_, const std::vector<int>& p_indices_, | ||
| Teuchos::RCP<Teuchos::ParameterList> params_ = Teuchos::null, Teuchos::EVerbosityLevel verbLevel= Teuchos::VERB_HIGH) : |
There was a problem hiding this comment.
Should the default verbosity level be high?
There was a problem hiding this comment.
I was debating about this. I thought that when one tries it out, without knowing all the options, it's good to have more info in output. For real applications, one can change the verbose level as preferred. Anyway, it's not going to affect performance, it's just many print outs (I'm not printing out, e.g. the entries of a vector). I can change if you disagree with this.
| { | ||
| outArgs.set_W_op(lop); | ||
| thyra_model.evalModel(inArgs, outArgs); | ||
| outArgs.set_W_op(Teuchos::null); | ||
| inArgs.set_x(Teuchos::null); | ||
| jac1 = lop; | ||
| updateJacobian1 = false; | ||
|
|
||
| computeJacobian1 = false; |
There was a problem hiding this comment.
The use of computeJacobian1 looks like it is introducing "state" into the model evaluator. Is that the case? It is something we have tried to avoid in the design.
There was a problem hiding this comment.
Unfortunately yes. I don't see an easy way to overcome this. Alternatively I could write this info in the parameter list provided by the application.. but it seems even less clean to me.
Note that if we used something like an observer object provided by, e.g., Piro, then we would need ROL to depend on Piro.. which is bad. The alternative is to move the Thyra adapters into Piro. Which could be a better solution.
I know that ROL is going to refactor the way the update function works. Not sure that will help (@dridzal ).
| if(params != Teuchos::null) | ||
| params->set<int>("Optimizer Iteration Number", iter); | ||
| } | ||
|
|
||
| /** \brief Update constraint functions with respect to Opt variable. | ||
| x is the optimization variable, | ||
| flag = true if optimization variable is changed, | ||
| flag = ??, |
There was a problem hiding this comment.
Does this doxygen need to be updated?
There was a problem hiding this comment.
Sorry.. at some point I thought that the flag was signalling whether the optimization variable is changed or not. However, it has a more subtle meaning and it does not appear to be consistent.
ROL team is going to completely refactor the udate function, that's why I left the ?? rather then trying to understand this better. Anyway, I'm going to work more on this as we add support for second derivatives.
| diff->set(*rol_z_ptr); | ||
| diff->axpy( -1.0, rol_z ); | ||
| Real norm = diff->norm(); | ||
| changed = (norm == 0) ? false : true; |
There was a problem hiding this comment.
Should this check have a tolerance due to roundoff?
There was a problem hiding this comment.
No.. I really want the two fields to be exactly the same, in which case diff is a vector of zeros and norm will be zero with no roundoff errors.
| diff->set(*rol_u_ptr); | ||
| diff->axpy( -1.0, rol_u ); | ||
| Real norm = diff->norm(); | ||
| changed = (norm == 0) ? false : true; |
| if (Teuchos::nonnull(rol_x_ptr)) { | ||
| rol_x_ptr->axpy( -1.0, rol_x ); | ||
| Real norm = rol_x_ptr->norm(); | ||
| changed = (norm == 0) ? false : true; |
| diff->set(*rol_z_ptr); | ||
| diff->axpy( -1.0, rol_z ); | ||
| Real norm = diff->norm(); | ||
| changed = (norm == 0) ? false : true; |
| diff->set(*rol_u_ptr); | ||
| diff->axpy( -1.0, rol_u ); | ||
| Real norm = diff->norm(); | ||
| changed = (norm == 0) ? false : true; |
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ rppawlo ]! |
|
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
1 similar comment
|
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
@trilinos/rol
Motivation
Improve performance by carefully avoiding re-computation of the objective, residual and their gradients when the state and/or control variables are not updated.
improved output messages (mainly for debugging purposes).
Stakeholder Feedback
Testing
Changes are tested in Piro test (collection of tests)
Piro_AnalysisDriverTpetra.