Skip to content

Conversation

@XueSongTap
Copy link
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)

@conradsnicta
Copy link
Contributor

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

Copy link
Member

@rcurtin rcurtin left a comment

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.

@rcurtin
Copy link
Member

rcurtin commented Sep 23, 2025

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

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Thanks again @XueSongTap 👍

Copy link

@github-actions github-actions bot left a comment

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