Skip to content

Refactor calculatedObjectives in NSGA2 optimizer - #434

Merged
rcurtin merged 3 commits into
mlpack:masterfrom
XueSongTap:master
Sep 25, 2025
Merged

Refactor calculatedObjectives in NSGA2 optimizer#434
rcurtin merged 3 commits into
mlpack:masterfrom
XueSongTap:master

Conversation

@XueSongTap

Copy link
Copy Markdown
Contributor

related: #433

Refactor NSGA2 calculatedObjectives from std::vectorarma::Col to arma::Cube

Changes

  • Replaced std::vector<arma::Col<ElemType>> with an arma::Cube<ElemType> for calculatedObjectives.

    • Layout: (numObjectives x 1 x populationSize)
    • Each slice corresponds to the objectives of one individual.
  • Updated all related methods (EvaluateObjectives, FastNonDominatedSort, Dominates, CrowdingDistanceAssignment, etc.) to work with the cube representation.

Compatibility

  • Public API is unchanged.
  • Internally, calculatedObjectives is now a cube instead of a vector of columns.
  • Existing tests build and run successfully after the refactor.
yxc@yxc-MS-7B89:~/code/2508/ensmallen/build$ ./ensmallen_tests
ensmallen version: 2.22.2 (E-Bike Excitement)
armadillo version: 12.6.7 (Cortisol Retox)
random seed: 0
===============================================================================
All tests passed (12616 assertions in 341 test cases)

Signed-off-by: yexiaochuan <tap91624@gmail.com>
@conradsnicta

Copy link
Copy Markdown
Contributor

@rcurtin Should this go in before ensmallen 3 is released? Related issue: #433

@rcurtin rcurtin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @XueSongTap, thank you for the contribution! I have some comments. If you can handle them we can get this merged.

Comment thread include/ensmallen_bits/nsga2/nsga2.hpp Outdated
Comment thread include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated
Comment thread include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated
@rcurtin

rcurtin commented Sep 23, 2025

Copy link
Copy Markdown
Member

I handled all the comments and merged in the master branch. If the tests pass I think this is good to go.

@rcurtin rcurtin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks again @XueSongTap 👍

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@rcurtin
rcurtin merged commit b22e53f into mlpack:master Sep 25, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants