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] Integration of NLOPT libraray #11349

Merged
merged 9 commits into from
Aug 8, 2023
Merged

Conversation

RezaNajian
Copy link
Contributor

This PR introduces an interface to the NLOPT optimizers. Currently, it supports three gradient-based optimizers: MMA, SLSQP, and LBFGS. The interface is designed in a flexible manner to ensure gradient-free optimizers can also be integrated with ease. However, our current focus is on gradient-based methods.

The interface is implemented at the Python level, and the NLOPT Python library needs to be in the Python path.

A shell thickness optimization case has been added and tested using the MMA algorithm.

@RezaNajian RezaNajian self-assigned this Jul 3, 2023
@RezaNajian RezaNajian requested review from a team as code owners July 3, 2023 14:42
Copy link
Member

@Igarizza Igarizza left a comment

Choose a reason for hiding this comment

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

@RezaNajian I am happy to see that your and @sunethwarna design allow this clean implementation of the external opt. library. In general, the PR is nice and clean. Still, I think there are places for further improvement.

@kratos_unittest.skipIf(not nlopt_available, "Missing nlopt python libraries ")
class TestNLOPTOptimizers(kratos_unittest.TestCase):
# @classmethod
# def setUpClass(cls):
Copy link
Member

Choose a reason for hiding this comment

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

left comments


# nlopt algorithm
self.algorithm_name = NLOPT_settings["algorithm_name"].GetString()
if(len(self.__constraints)==0):
Copy link
Member

Choose a reason for hiding this comment

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

can you update the CheckOptimizerSupport to take _optimization_problem and do all the checks inside to keep init cleaner?

@@ -0,0 +1,124 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

please move NLOPT_tests folder into algorithm_test.

@@ -203,7 +203,7 @@ def Update(self, new_control_field: ContainerExpressionTypes) -> bool:
if not IsSameContainerExpression(new_control_field, self.GetEmptyField()):
raise RuntimeError(f"Updates for the required element container not found for control \"{self.GetName()}\". [ required model part name: {self.model_part.FullName()}, given model part name: {new_control_field.GetModelPart().FullName()} ]")

if KratosOA.ExpressionUtils.NormL2(self.control_field - new_control_field) > 1e-9:
if KratosOA.ExpressionUtils.NormL2(self.control_field - new_control_field) > 1e-15:
Copy link
Member

Choose a reason for hiding this comment

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

Should we move this tolerance into JSON file to allow changing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not know honestly, I am open to suggestions

@RezaNajian RezaNajian merged commit 8e1274d into master Aug 8, 2023
@RezaNajian RezaNajian deleted the optapp/NLOPT-integration branch August 8, 2023 17:43
@RezaNajian RezaNajian restored the optapp/NLOPT-integration branch August 15, 2023 09:12
@RezaNajian
Copy link
Contributor Author

this PR and the corresponding reviews are moved and addressed in PR #11480.

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.

3 participants