[OptApp] Introducing AlgorithmDataManager to store and retrieve data.

[talhah-ansari]

📝 Description

This PR introduces a new AlgorithmDataManager utility class to centralize and standardize the management of algorithm data within the optimization algorithms. The affected algorithms have been refactored to use this new class for storing and retrieving data.

🆕 Changelog

• Added new AlgorithmDataManager class in algorithm_data_manager.py to encapsulate logic for storing and retrieving algorithm-related data.
• Refactored AlgorithmGradientProjection, AlgorithmRelaxedGradientProjection, AlgorithmMomentumRelaxedGradientProjection, and AlgorithmNesterovAccelaratedGradient to use AlgorithmDataManager for all algorithm data interactions, replacing previous direct usage of ComponentDataView and BufferedDict.
• Updated algorithm constructors to initialize the list of control names and store them in the unbuffered data, ensuring consistent access to control-related information.

[Copilot]

🟡 Changes recommended

Critical and moderate issues in AlgorithmDataManager remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR centralizes algorithm data storage and retrieval through AlgorithmDataManager and updates related optimization algorithms and tests.

Changes:

• Added per-control algorithm data management.
• Refactored algorithms and line-search utilities.
• Updated tests and expected output summaries.

File summaries

File	Summary
applications/OptimizationApplication/tests/algorithm_tests/test_line_search.py	Updated line-search data fixtures.
applications/OptimizationApplication/tests/algorithm_tests/analysis_based_tests/algorithm_steepest_descent/summary_orig.csv	Updated expected control metadata.
applications/OptimizationApplication/tests/algorithm_tests/analysis_based_tests/algorithm_steepest_descent_qnbb/summary_orig.csv	Updated expected control metadata.
applications/OptimizationApplication/tests/algorithm_tests/analysis_based_tests/algorithm_nesterov_accelerated_gradient/summary_orig.csv	Updated expected control metadata.
applications/OptimizationApplication/python_scripts/utilities/opt_line_search.py	Uses managed algorithm data.
applications/OptimizationApplication/python_scripts/algorithms/algorithm_steepest_descent.py	Refactored algorithm data access.
applications/OptimizationApplication/python_scripts/algorithms/algorithm_relaxed_gradient_projection.py	Refactored algorithm data access.
applications/OptimizationApplication/python_scripts/algorithms/algorithm_nesterov_accelarated_gradient.py	Uses centralized data management.
applications/OptimizationApplication/python_scripts/algorithms/algorithm_momentum_relaxed_gradient_projection.py	Uses centralized data management.
applications/OptimizationApplication/python_scripts/algorithms/algorithm_gradient_projection.py	Refactored algorithm data access.
applications/OptimizationApplication/python_scripts/algorithms/algorithm_data_manager.py	Implements centralized per-control data management.

Review details

Suppressed comments (2)

applications/OptimizationApplication/python_scripts/algorithms/algorithm_data_manager.py:21

• Storing each combined tensor only as per-control children changes the value returned by the existing GetComponentValueByFullName API: search_direction:historical now resolves to a BufferedDict, not an object with .data. L2ConvCriterion still reads that key directly and will raise "is not a field" when configured with any of these algorithms. Preserve a compatible aggregate value or update the convergence consumer to use this manager.

            for control_name, tensor_adaptor in zip(self.__list_of_control_names, value.GetTensorAdaptors()):
                data_view.SetValue(f"{key}/{control_name}_{key}", tensor_adaptor, step_index, overwrite)

applications/OptimizationApplication/tests/algorithm_tests/test_line_search.py:33

• This fixture writes a combined adaptor directly into each per-control leaf, whereas AlgorithmDataManager.SetValue stores value.GetTensorAdaptors() (one adaptor per control). With only one control, the nested combined adaptor happens to expose the same flat data, so this does not exercise the new decomposition/reconstruction contract or multi-control history. Exercise SetValue/GetValue with at least two controls or add a dedicated manager test.

        ComponentDataView("algorithm", cls.optimization_problem).GetBufferedData()["search_direction/test_control_search_direction"] = search_direction
        ComponentDataView("algorithm", cls.optimization_problem_one_step).GetBufferedData()["search_direction/test_control_search_direction"] = sensitivity
        ComponentDataView("algorithm", cls.optimization_problem).GetBufferedData()["control_field_update/test_control_control_field_update"] = control_field_update

• Files reviewed: 11/11 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Igarizza]

@talhah-ansari can you please explain what does it improve? I can't understand

[Igarizza]

Suggestion: name the fields per control at output time instead of adding AlgorithmDataManager

I may have the motivation wrong, so please correct me. My guess is that the problem is field output with more than one control. Say a Young's modulus control and a density control act on the same elements. Both sub-adaptors of search_direction then get the same output name, because CombinedTensorAdaptorData.GetTensorAdaptorName() returns only the last segment of the data path. VTU output then fails with "Found an existing data array with the same name".

If that's the issue, I think we can fix it where the name is created and leave the algorithm data layout unchanged. OutputGradientFields already handles gradients this way, by building d<response>_d<control> names when writing output.

utilities/optimization_problem_utilities.py

def GetCombinedTensorAdaptorNames(optimization_problem: OptimizationProblem, combined_tensor_adaptor_path: str, combined_tensor_adaptor: Kratos.TensorAdaptors.DoubleCombinedTensorAdaptor) -> 'list[str]':
    base_name = combined_tensor_adaptor_path[combined_tensor_adaptor_path.rfind("/")+1:]
    sub_tensor_adaptors = combined_tensor_adaptor.GetTensorAdaptors()

    # single control: keep the current names, so existing reference outputs stay valid
    if len(sub_tensor_adaptors) == 1:
        return [base_name]

    # there is always exactly one master control in the problem
    master_control, = optimization_problem.GetListOfMasterControls()
    controls = master_control.GetListOfControls()
    if len(controls) == len(sub_tensor_adaptors) and all(control.GetEmptyField().GetContainer() == sub_ta.GetContainer() for control, sub_ta in zip(controls, sub_tensor_adaptors)):
        return [f"{control.GetName()}_{base_name}" for control in controls]

    # not in control space: unique names by index
    return [f"{base_name}_{i}" for i in range(len(sub_tensor_adaptors))]

processes/optimization_problem_field_output_process.py, in InitializeVtuOutputIO. This runs once, and the HDF5 output inherits it:

if isinstance(global_v, Kratos.TensorAdaptors.DoubleCombinedTensorAdaptor):
    names = GetCombinedTensorAdaptorNames(self.optimization_problem, global_k, global_v)
    for i, name in enumerate(names):
        self.__AddTensorAdaptor(CombinedTensorAdaptorData(global_k, global_v, i, name))

Why I'd prefer this to a new data-manager layer

• Algorithms, line search, convergence criteria, the summary CSV and restart are all unchanged. Each key still holds one DoubleCombinedTensorAdaptor.
• It fixes the collision for every combined adaptor in the data container, not only the ones algorithms write. Responses and controls are covered as well.
• ADAM, SciPy, NLopt and l2_conv_criterion (default field search_direction:historical) keep working with no changes. As it stands, the PR breaks ADAM (self.algorithm_data was removed) and the L2 criterion (it now finds a sub-dict instead of a field).
• HasValue behaves as before. With nested keys it returns True for the parent after AdvanceStep, so for example QNBBStep stores step_size only at step 0.
• No hidden "controls" string and no aliasing between stored sub-adaptors and the caller's combined adaptor.

Test I'd add: two controls on the same element container (e.g. YOUNG_MODULUS and DENSITY) with VTU output of the algorithm data. Check that <control>_search_direction arrays are written for both controls.

@sunethwarna can smth like this be enough? or I miss smth ?

[Igarizza]

The kratos-reviewer recommends request changes on PR #14676 ("[OptApp] Introducing AlgorithmDataManager…"). It could only build or run one thing: a Python 3.11 compile of the PR's file, which confirmed blocker 1. Everything else comes from reading the code.

The branch has already been reworked along the lines of your 2026-10-09 comment, so AlgorithmDataManager is gone. What it does now:

• Each algorithm stores its control names as one comma-joined string, "controls", in the algorithm data.
• OptimizationProblemFieldOutputProcess reads that string back and names sub-adaptor i of every combined tensor adaptor <controls[i]>_<key>.

Blockers

1. The module doesn't import on Python older than 3.12. In optimization_problem_utilities.py:160, the f-string reuses " inside itself, which needs PEP 701 (3.12). Python 3.11 rejects it with a SyntaxError. CI uses 3.10 and 3.8, and every output process imports this module, so every OptApp/SysId run would fail at import. Fix: f"{self.prefix}_{super().GetTensorAdaptorName()}".
2. Control names are attached to fields that aren't per-control. The output process applies the prefix to every combined adaptor, including physical-variable gradients like d<resp>_d<VAR>.
   • Example: with control A on YOUNG_MODULUS and control B on THICKNESS, the dresp_dTHICKNESS field gets written as A_dresp_dTHICKNESS.
   • An adaptor with more sub-adaptors than there are controls raises IndexError.
   • The PR's own regenerated test references already show this: names like control_2_resp_1_combined_element_True.
3. Output now requires the "controls" key, which only built-in algorithms write. The lookup runs unconditionally (optimization_problem_field_output_process.py:68). Custom "module" algorithms and scripts that build the problem directly crash at the first PrintOutput.

Should-fix

• Stale call: TensorAdaptorDataManager.InitializeHDF5Output (line 204) still uses the old CombinedTensorAdaptorData signature, so it would raise TypeError. Nothing calls that class today.
• Single-control runs change names for no gain: they now write material_control_search_direction instead of search_direction, etc. That breaks users' ParaView states and scripts, and it's why four summary_orig.csv files and the VTU references were regenerated.
• No test of the case that motivated the PR: two real controls on the same container, checking that both search_direction fields come out under the right names.
• Out-of-date description: the PR title and description still describe the removed AlgorithmDataManager design.

Nits: the "controls" line is copied into five algorithm files; a comma-joined string breaks if a control name contains a comma; there's a stray blank line at line 81; prefix has no docstring.

Suggested direction (same as your earlier comment): take the names from optimization_problem.GetListOfMasterControls() in the output process. Add a control's name only when the sub-adaptors line up one-to-one with the controls (same count and matching containers). Keep the plain <key> when there's only one sub-adaptor. That fixes all three blockers, and the existing reference files would no longer need regenerating.

To read the code, the reviewer fetched the PR into $KRATOS_ROOT/master as refs/remotes/pr/14676. It didn't check anything out or modify the worktree. Delete the ref with git update-ref -d refs/remotes/pr/14676 if you don't want it.

[talhah-ansari]

@sunethwarna thanks. Is this CombinedTensorAdaptorData initialization still valid? Shouldn't there be 4 arguments?

[sunethwarna]

good catch... This is not used anywhere yet. Thats why it did not lead to failure in the unit tests.... I will fix it

[Igarizza]

@sunethwarna @talhah-ansari Nice work! I am happy with this version :)

[Igarizza]

Description would be helpful