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

[OptApp] Small fixes to RGP, Conv Crit, NLOPT test #12844

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Igarizza
Copy link
Member

📝 Description
This pull request includes several changes aimed at improving the convergence criteria, logging, and testing in the OptimizationApplication. The key changes include updates to convergence checks, enhanced logging for control updates, and improvements in test configurations.

🆕 Changelog

Convergence Criteria Improvements:

  • Modified the Solve method in algorithm_gradient_projection.py and algorithm_relaxed_gradient_projection.py to include additional convergence checks and handle maximum iterations reached ([[1]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-b969e5c1475ed205b65703b1bc60ff16b6238211dc61bb9b29911bb337e9fa1fL214-R218), [[2]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-01c0c38f43755b39d627f69f1b3d3fe2d0c55bb30faa5853c548d69ecca44695L297-R301)).
  • Added the IsMaxIterationsReached method to the opt_convergence.py file to check if the maximum number of iterations has been reached ([[1]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-53198d3789dc3a41e658f177c6adcac323a55abae9fad3923b83494953660b1bR37-R39), [[2]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-53198d3789dc3a41e658f177c6adcac323a55abae9fad3923b83494953660b1bR70-R72), [[3]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-53198d3789dc3a41e658f177c6adcac323a55abae9fad3923b83494953660b1bR117-R119), [[4]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-53198d3789dc3a41e658f177c6adcac323a55abae9fad3923b83494953660b1bR173-R175), [[5]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-53198d3789dc3a41e658f177c6adcac323a55abae9fad3923b83494953660b1bR217-R219)).

Logging Enhancements:

  • Added logging to indicate whether a control has been updated in the Update method of master_control.py ([applications/OptimizationApplication/python_scripts/controls/master_control.pyR185-R188](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-394f78969e3543309c73a5a5f34fc4229043774270ff1f62f857426656117651R185-R188)).

Testing Improvements:

  • Updated test configurations and added a comparison process for the optimization log in test_mma_optimizer.py ([[1]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-1e1b3d0242ca9473d075c475a6be65e3986841b7754ca6c70006e7b147868b2cR5-R8), [[2]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-1e1b3d0242ca9473d075c475a6be65e3986841b7754ca6c70006e7b147868b2cR22-R38)).
  • Adjusted material properties and optimization parameters in the test files to reflect the new configurations ([[1]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-3d52cbac5abb020f3482e77973c6af2dfca69d4ce3594cd677847711175ada6aL14-R14), [[2]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-fe0085ab276aa4c5ed8a57b7623566fc38b68c1fe8c194c44b52246b970602d5L64-R79), [[3]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-fe0085ab276aa4c5ed8a57b7623566fc38b68c1fe8c194c44b52246b970602d5L96-R90), [[4]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-fe0085ab276aa4c5ed8a57b7623566fc38b68c1fe8c194c44b52246b970602d5L123-R117), [[5]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-31a83a4861af6d6b4de3d953b5597af3ab218deb1ffd743131260b4a9c8e983fL5-R7)).

Additional Changes:

  • Imported the math module in vertex_morphing_shape_control.py and used math.isclose for numerical comparisons ([[1]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-8bc62836048115abfbb5078bea9f1bcbeccb86af0b0887381a3768de7dec3d94R14), [[2]](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-8bc62836048115abfbb5078bea9f1bcbeccb86af0b0887381a3768de7dec3d94L150-R152)).
  • Added a summary output file for the optimization problem ([applications/OptimizationApplication/tests/algorithm_tests/nlopt_tests/mma_shell_thickness_opt/summary_orig.csvR1-R18](https://github.com/KratosMultiphysics/Kratos/pull/12844/files#diff-80bdf6f073ebcd30439349c4aa938b6e2de826ec702e7dd4bd0aba5c21bec472R1-R18)).

@Igarizza Igarizza requested a review from sunethwarna November 12, 2024 12:44
@Igarizza Igarizza self-assigned this Nov 12, 2024
Copy link
Member

@sunethwarna sunethwarna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Igarizza. I have few comments, otherwise this is OK for me.

Comment on lines +185 to +188
if update_map[control]:
Kratos.Logger.PrintInfo(f"Control {control.GetName()} updated.")
else:
Kratos.Logger.PrintInfo(f"Control {control.GetName()} not updated")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

Comment on lines +25 to +44
"""
A convergence criterion class that checks if the maximum number of iterations has been reached.

Methods
-------
GetDefaultParameters():
Returns the default parameters for the convergence criterion.

__init__(parameters: Kratos.Parameters, optimization_problem: OptimizationProblem):
Initializes the convergence criterion with the given parameters and optimization problem.

IsMaxIterationsReached():
Checks if the maximum number of iterations has been reached.

IsConverged(search_direction=None) -> bool:
Checks if the optimization problem has converged based on the maximum number of iterations.

GetInfo() -> dict:
Returns a dictionary with information about the current state of convergence.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matekelemen, does this way of writing method docs within the class doc works? can doxygen decipher this and put it to correct method?

Igarizza and others added 2 commits November 19, 2024 14:36
@Igarizza
Copy link
Member Author

Igarizza commented Dec 9, 2024

@sunethwarna can you check this PR please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants