Skip to content

Pareto: Genereate unique centroids, documentation, test#177

Merged
timokau merged 4 commits intokiudee:masterfrom
timokau:pareto-fixup
Nov 20, 2020
Merged

Pareto: Genereate unique centroids, documentation, test#177
timokau merged 4 commits intokiudee:masterfrom
timokau:pareto-fixup

Conversation

@timokau
Copy link
Copy Markdown
Collaborator

@timokau timokau commented Nov 19, 2020

Description

The Pareto problem generation re-used the same centroid for 10 times (which seems to be a bug) and generated invalid elements when n_instances was not a multiple of 10. This tries to fix these problems an adds some documentation on the things @kiudee cleared up for me. This is a result of #164 (comment).

How Has This Been Tested?

Ran the new test, relying on CI for the rest.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@timokau timokau changed the title Pareto fixup Pareto: Genereate unique centroids, documentation, test Nov 19, 2020
I think default values for internal functions just hinder understanding.
Changed the parameter names to be less domain specific, since we are
just talking about a point in the ball for the purposes of this
function. Since this is an internal function, we can require an already
initialized random state.

Result of this discussion / explanation:
kiudee#164 (comment)
Thereby fixing a bug when the number of instances is not a multiple of
10.

Result of this discussion
kiudee#164 (comment)
Default values for internal functions just add confusion. The parameter
names had conflicting meaning, so I switched them to less
domain-specific ones. We can assume random states are initialized in
internal functions. We only ever need to generate a single instance in
this function, so there is no need for multi-instance support.
Just a small sanity check to at least exercise the functionality and
verify it does something sensible.
Copy link
Copy Markdown
Owner

@kiudee kiudee left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@timokau timokau merged commit f266087 into kiudee:master Nov 20, 2020
@timokau timokau deleted the pareto-fixup branch November 20, 2020 11:34
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.

2 participants