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

Error from empty set_population_size() call in PopulationBasedOptimiser #1690

Open
mjowen opened this issue Nov 14, 2024 · 1 comment
Open
Assignees
Labels

Comments

@mjowen
Copy link

mjowen commented Nov 14, 2024

Bug

Refusing to set a population size (using the default of population_size=None) leads to an error when the optimisation is run but the docstring for set_population_size suggests this is valid and the value generated by suggested_population_size should be used in this case.

I'm guessing this is just missing functionality from set_population_size, but I did see that the docstring for population_size mentions the possibility of self.population_size=None prior to beginning an optimisation, but I couldn't see any way for opt.run() to set this to default if that was the case

MRE

import pints
import pints.toy

model = pints.toy.ParabolicError()
opt = pints.OptimisationController(model, [1, 1], method=pints.CMAES)
opt.optimiser().set_population_size()
opt.run()

Relevant parts of pints

    def __init__(self, x0, sigma0=None, boundaries=None):
        super(PopulationBasedOptimiser, self).__init__(x0, sigma0, boundaries)

        # Set initial population size using heuristic
        self._population_size = self._suggested_population_size()

    def population_size(self):
        """
        Returns this optimiser's population size.

        If no explicit population size has been set, ``None`` may be returned.
        Once running, the correct value will always be returned.
        """
        return self._population_size

    def set_population_size(self, population_size=None):
        """
        Sets a population size to use in this optimisation.

        If `population_size` is set to ``None``, the population size will be
        set using the heuristic :meth:`suggested_population_size()`.
        """
        if self.running():
            raise Exception('Cannot change population size during run.')

        # Check population size or set using heuristic
        if population_size is not None:
            population_size = int(population_size)
            if population_size < 1:
                raise ValueError('Population size must be at least 1.')

        # Store
        self._population_size = population_size

    def suggested_population_size(self, round_up_to_multiple_of=None):
        """
        Returns a suggested population size for this method, based on the
        dimension of the search space (e.g. the parameter space).

        If the optional argument ``round_up_to_multiple_of`` is set to an
        integer greater than 1, the method will round up the estimate to a
        multiple of that number. This can be useful to obtain a population size
        based on e.g. the number of worker processes used to perform objective
        function evaluations.
        """
        population_size = self._suggested_population_size()

        if round_up_to_multiple_of is not None:
            n = int(round_up_to_multiple_of)
            if n > 1:
                population_size = n * (((population_size - 1) // n) + 1)

        return population_size
mjowen added a commit to CardiacModelling/ionBench that referenced this issue Nov 14, 2024
pints doesn't like popSize=None
pints-team/pints#1690
@MichaelClerx MichaelClerx self-assigned this Dec 18, 2024
@MichaelClerx
Copy link
Member

Thanks @mjowen ! Will look into this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants