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

Avoiding unnecessary setup operations #503

Merged
merged 8 commits into from
Jan 18, 2024
Merged
39 changes: 6 additions & 33 deletions src/fastoad/io/configuration/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,15 @@
from importlib.resources import open_text
from typing import Dict

import numpy as np
import openmdao.api as om
import tomlkit
from jsonschema import validate
from ruamel.yaml import YAML

from fastoad._utils.files import make_parent_dir
from fastoad.io import DataFile, IVariableIOFormatter
from fastoad.io import IVariableIOFormatter
from fastoad.module_management.service_registry import RegisterOpenMDAOSystem, RegisterSubmodel
from fastoad.openmdao.problem import FASTOADProblem
from fastoad.openmdao.variables import VariableList
from . import resources
from .exceptions import (
FASTConfigurationBadOpenMDAOInstructionError,
Expand Down Expand Up @@ -115,6 +113,10 @@

problem = FASTOADProblem()
self._build_model(problem)
Comment on lines 114 to 115
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not a change in the PR but I'm wondering here if this line means that we rebuild the problem each time we call the get_problem method. For instance if we were to keep calling the write_needed_inputs from the FASTOADProblemConfigurator class we would construct the problem twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, the problem is built each time get_problem is called. I think it's better like this because:

  • if a user called get_problem, he is able to keep the generated instance, and I think he has no reason to call get_problem again unless he wants a fresh new instance.
  • we could imagine a case where the configuration is read, get_problem is called, then the configuration is programmatically modified (e.g. optimization definition) and get_problem is called again. In such case, a fresh new instance is really better.

This is why write_needed_inputs implementation has been moved to the FASTOADProblem class, which allows doing this operation on the instance that will be used for the computation.


if self._configuration_modifier:
self._configuration_modifier.modify(problem)

Check warning on line 118 in src/fastoad/io/configuration/configuration.py

View check run for this annotation

Codecov / codecov/patch

src/fastoad/io/configuration/configuration.py#L118

Added line #L118 was not covered by tests

problem.input_file_path = self.input_file_path
problem.output_file_path = self.output_file_path

Expand All @@ -132,9 +134,6 @@
if read_inputs:
self._add_design_vars(problem.model, auto_scaling)

if self._configuration_modifier:
self._configuration_modifier.modify(problem)

return problem

def load(self, conf_file):
Expand Down Expand Up @@ -213,33 +212,7 @@
not provided, expected format will be the default one.
"""
problem = self.get_problem(read_inputs=False)
problem.setup()
variables = DataFile(self.input_file_path, load_data=False)

unconnected_inputs = VariableList.from_problem(
problem,
use_initial_values=True,
get_promoted_names=True,
promoted_only=True,
io_status="inputs",
)

variables.update(
unconnected_inputs,
add_variables=True,
)
if source_file_path:
ref_vars = DataFile(source_file_path, formatter=source_formatter)
variables.update(ref_vars, add_variables=False)
nan_variable_names = []
for var in variables:
var.is_input = True
# Checking if variables have NaN values
if np.any(np.isnan(var.value)):
nan_variable_names.append(var.name)
if nan_variable_names:
_LOGGER.warning("The following variables have NaN values: %s", nan_variable_names)
variables.save()
problem.write_needed_inputs(source_file_path, source_formatter)

def get_optimization_definition(self) -> Dict:
"""
Expand Down
49 changes: 27 additions & 22 deletions src/fastoad/openmdao/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Utility functions for OpenMDAO classes/instances
"""
# This file is part of FAST-OAD : A framework for rapid Overall Aircraft Design
# Copyright (C) 2023 ONERA & ISAE-SUPAERO
# Copyright (C) 2024 ONERA & ISAE-SUPAERO
# FAST is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
Expand All @@ -16,52 +16,57 @@

from contextlib import contextmanager
from copy import deepcopy
from typing import List, Tuple
from typing import List, Tuple, TypeVar

import numpy as np
import openmdao.api as om
from deprecated import deprecated
from openmdao.core.constants import _SetupStatus
from openmdao.utils.mpi import FakeComm

T = TypeVar("T", bound=om.Problem)

@contextmanager
def problem_without_mpi(problem: om.Problem) -> om.Problem:
"""
Context manager that delivers a copy of the given OpenMDAO problem.

A deepcopy operation may crash if problem.comm is not pickle-able, like a
def get_mpi_safe_problem_copy(problem: T) -> T:
"""
This function does a deep copy of input OpenMDAO problem while avoiding
the crash that can occur if problem.comm is not pickle-able, like a
mpi4py.MPI.Intracomm object.

This context manager temporarily sets a FakeComm object as problem.comm and
does the copy.
:param problem:
:return: a copy of the problem with a FakeComm object as problem.comm
"""
with copyable_problem(problem) as no_mpi_problem:
problem_copy = deepcopy(no_mpi_problem)

return problem_copy


It ensures the original problem gets back its original communicator after
@contextmanager
def copyable_problem(problem: om.Problem) -> om.Problem:
"""
Context manager that temporarily makes the input problem compatible with deepcopy.

It ensures the problem gets back its original attributes after
the `with` block is ended.

:param problem: any openMDAO problem
:return: A copy of the given problem with a FakeComm object as problem.comm
:return: The given problem with a FakeComm object as problem.comm
"""

# An actual MPI communicator will make the deepcopy crash if an MPI
# library is installed.

actual_comm = problem.comm
problem.comm = FakeComm()

metadata_were_added = False
try:
if not problem._metadata:
# Adding temporarily this attribute ensures that the post-hook for N2 reports
# Adding this attribute ensures that the post-hook for N2 reports
# will not crash. Indeed, due to the copy, it tries to post-process
# the 'problem' instance at the end of setup of the 'problem_copy' instance.
problem._metadata = {"saved_errors": []}
metadata_were_added = True
problem_copy = deepcopy(problem)
problem_copy.comm = problem.comm

yield problem_copy
problem._metadata = {"saved_errors": [], "setup_status": _SetupStatus.PRE_SETUP}

Check warning on line 67 in src/fastoad/openmdao/_utils.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/fastoad/openmdao/_utils.py#L67

Access to a protected member _metadata of a client class
yield problem
finally:
if metadata_were_added:
problem._metadata = None
problem.comm = actual_comm


Expand Down
Loading