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

Add cost function decorator #226

Merged
merged 3 commits into from
Aug 20, 2018
Merged

Add cost function decorator #226

merged 3 commits into from
Aug 20, 2018

Conversation

whzup
Copy link
Collaborator

@whzup whzup commented Aug 15, 2018

Description

This decorator allows the creation of much simpler cost functions. Instead of writing a cost function that returns a shape of (n_particles, 0) it enables the usage of shorter and simpler cost functions that directly return the cost.

Related Issue

Resolves: #228

Motivation and Context

At the moment, the way one has to write the cost function is very counterintuitive as it is necessary to write a function that returns an array of shape (n_particles, ). This decorator "vectorizes" a cost function which returns just the cost.

How Has This Been Tested?

Wrote a new test file.

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a 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 read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests pass.

@whzup whzup added enhancement Feature requests documentation Documentation improvements or fixes unit tests Test-related changes v.1.1.0 In pipeline for next version labels Aug 15, 2018
@whzup whzup requested a review from ljvmiranda921 August 15, 2018 17:52
:code:`int`.

Attributes
==========
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should be a second-level header, and Parameters:

def cost(cost_func):
    """

    Parameters
    ----------
    """

Same with Returns 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, thanks for the catch 😄

@ljvmiranda921
Copy link
Owner

ljvmiranda921 commented Aug 17, 2018

I'll update this branch first (not sure if you can click the button over there) so that #229 will also reflect here:

screenshot from 2018-08-17 09-19-50


@pytest.mark.parametrize(
"objective_func",
[np.sum(), np.prod()]
Copy link
Owner

@ljvmiranda921 ljvmiranda921 Aug 17, 2018

Choose a reason for hiding this comment

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

Just pass the function reference: np.sum and np.prod

Can you also try np.dot? Say you're given a particle with d dimensions, and you dot-product (sumproduct) it to an objective function (randomly-generated array with d dimensions), you only get 1 output. With the @cost decorator, you can run this on a set of n particles with d dimensions too 👍 This workflow is similar to our neural network example, so we might as well try 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd have to define a new function for this since np.dot takes two arguments. Shall I put it in the conftest.py file?

Parameters
----------

cost_func : function
Copy link
Owner

Choose a reason for hiding this comment

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

This should be callable instead of function

----------

cost_func : function
A function that can be used as cost function in the optimization (must return a :code:`float` or an :code:`int`)
Copy link
Owner

Choose a reason for hiding this comment

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

This line's a bit long (?) I don't think black formats docstrings, can flake8 catch this line? What does flake8 decorators.py say?

w_o_decorator = cost_func_w_o_decorator(particles)
w_decorator = cost_func_w_decorator(particles)

assert isinstance(w_decorator, Number)
Copy link
Owner

Choose a reason for hiding this comment

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

Hmmm, I'm not sure why we need to test if the output is a Number. Maybe we should test the shape of the resulting array? Ensure that it's (n_particles, )?

-------

cost_dec : function
The vectorized output for all particles as defined by :code:`cost_func`
Copy link
Owner

@ljvmiranda921 ljvmiranda921 Aug 17, 2018

Choose a reason for hiding this comment

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

This should be:

"""
Returns
----------
callable
    Put description here
"""

needs to resemble a strategy that can be applied to every particle in the swarm
to assign a cost. This means that the decorated cost function must use an array
of shape :code:`(dimensions, )` as an argument and return a :code:`float` or an
:code:`int`.
Copy link
Owner

Choose a reason for hiding this comment

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

I think this can be phrased as:

If you have defined a cost function that only acts upon a single particle, then you can use this decorator to have a vectorized computation for n particles. ...

Then give an example code-block maybe? I think good keywords here are: wrapper, transforms, etc. Strategy seems a bit odd (I think you got this from hypothesis haha but I think it's more of their local lingo).

"objective_func",
[np.sum(), np.prod()]
)
def test_cost_decorator(objective_func, particles):
Copy link
Owner

Choose a reason for hiding this comment

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

Where do you get the particles? I suggest you can generate them via np.random.uniform...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you know a way we can output the seed number with the tests without using pytest -s? In this way, we don't have fixed particle arrays and if it fails we can use the printed seed to debug it.

[np.sum(), np.prod()]
)
def test_cost_decorator(objective_func, particles):
def cost_func_w_o_decorator(x):
Copy link
Owner

Choose a reason for hiding this comment

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

It's ok to spell it out 😆 cost_func_without_decorator(x) and cost_fun_with_decorator

@ljvmiranda921
Copy link
Owner

Aside from my comments, I think this is great, @whzup ! This is an elegant and useful improvement to our current codebase! Very excited to try this out!

@whzup
Copy link
Collaborator Author

whzup commented Aug 17, 2018

@ljvmiranda921 Thanks for the comments hahaha. Would've been searching for ages to find a solution to these things 🙈

@whzup
Copy link
Collaborator Author

whzup commented Aug 17, 2018

Let's update the branch before we merge this one. I had some trouble with the rebasing of the commits.

# Import modules
import pytest
import numpy as np
import sys
Copy link
Owner

@ljvmiranda921 ljvmiranda921 Aug 18, 2018

Choose a reason for hiding this comment

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

I wonder where you'll use import sys for... 👨‍🚀

@whzup whzup changed the title [WIP] Add cost function decorator Add cost function decorator Aug 18, 2018
@whzup
Copy link
Collaborator Author

whzup commented Aug 18, 2018

Ok, I guess we're ready for the merge. I think we can add change the representation of cost functions in another PR. What do you think?

@ljvmiranda921
Copy link
Owner

Hi @whzup

I think we can add change the representation of cost functions in another PR. What do you think?

You mean the single_obj cost functions? I think there's no need to change them since they're all vectorized already.

Ok, I guess we're ready for the merge.

OK let me check first then I'll squash-merge this 👍

Copy link
Owner

@ljvmiranda921 ljvmiranda921 left a comment

Choose a reason for hiding this comment

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

Thank you for this @whzup ! Almost there! Just a few more things:

  • Please update docs/api/_pyswarms.utils.rst/. Add pyswarms.utils.decorators at the top (and since you're there, rearrange them alphabetically--decorators, functions, plotters, search)
  • Just a comment on __init__. The previous version is already OK. Just import cost explicitly for now. I predict this feature will be widely-used compared to other future decorators. It's better to load other decorators via pyswarms.utils.decorators

@@ -16,5 +16,8 @@

from .single import global_best, local_best, general_optimizer
from .discrete import binary
from .utils.decorators import *
from . import utils
Copy link
Owner

Choose a reason for hiding this comment

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

Hmmm, I think the previous version was enough where cost is defined explicitly.
I think the @constraint decorator in the future is better loaded whenever the user needs it.

import numpy as np


def cost(cost_func):
Copy link
Owner

Choose a reason for hiding this comment

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

💯 This is good already

"objective_func",
[np.sum, np.prod]
)
def test_cost_decorator(objective_func, particles):
Copy link
Owner

Choose a reason for hiding this comment

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

The tests are good already 💯

@whzup
Copy link
Collaborator Author

whzup commented Aug 18, 2018

Hmm, I see your point. Although, what do you think about the numpy approach? Almost everything in their package can be accessed by using np.<name>. I mean, for us it's pretty clear where everything is and the project isn't that big at the moment. But for people who aren't used to the structure of this project it might be very hard to find everything and if the project becomes bigger it might become very messy to import from all these different modules. They have to search through the docs and write all these sub-module names in their code which contributes to spaghetti code. Also, if we just import the __all__ attributes into the __init__.py file of the whole package the user can still use the sub-module names etc. it's just that it's also possible to just say pyswarms.<name>. So we'd just add another option for the users who want to have very little and lucid code.
The option with the explicit statement of the sub-modules might be useful for people who want to write their own optimisation loop and the option with pyswarms.<name> for people who just want to hack together a small optimisation script.

You mean the single_obj cost functions? I think there's no need to change them since they're all vectorized already.

No, I mean all cost functions in our examples 😄

@ljvmiranda921
Copy link
Owner

ljvmiranda921 commented Aug 18, 2018

Hi @whzup ,

Although, what do you think about the numpy approach? Almost everything in their package can be accessed by using np.

I thought about that before, that's why there's this presence of import pyswarms as ps that I "copied" from the numpy style. The only thing that's stopping me from doing this is that we'll have instances of this:

import pyswarms as ps

ps.single.GlobalBestPSO # Good enough
ps.utils.functions.single_obj.sphere # Pretty long
ps.utils.decorators.decorators.cost # Kinda long too..

If it is possible to do ps.sphere or ps.cost but still respect the modules themselves, then maybe we can do something like that. We can load everything right away, but I'm just wary that we might have conflicting methods or something in the future.

That's the reason why I decided to have everything be loaded whenever needed. Atleast I know that users will always come for the high-level optimizers (and I'm sure the cost decorator too), but they won't really use all objective functions, and might clutter our namespace.

Right now, the design kinda follows that of Keras, where we have components separated into modules, and we load them whenever needed. The thing about numpy is that instead of components, you have operators which you will use regularly. It would then be a hassle if numpy does something similar to ours.

Still open for this idea, but for now just explicitly declare cost, we can discuss this in the later 0.X versions (v.0.5.0) 👍

@ljvmiranda921
Copy link
Owner

No, I mean all cost functions in our examples 😄

Let's open-up an issue after I merge #227. We'd probably update some of our examples later + README (more animations haha)

whzup added 3 commits August 19, 2018 14:41
Added a cost function decorator which allows the user to write a cost
function that serves as a model for how the cost will be computed for
every one particle. The decorator is tested with a pytest file where the
shape and the equality with an example function is checked. I added a
note to the documentation that some numpy functions will return arrays
with single values in them.
@whzup
Copy link
Collaborator Author

whzup commented Aug 19, 2018

Done! 👍

Copy link
Owner

@ljvmiranda921 ljvmiranda921 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ljvmiranda921 ljvmiranda921 merged commit 0f7d9d1 into ljvmiranda921:development Aug 20, 2018
@whzup whzup deleted the decorators branch August 31, 2018 14:59
@ljvmiranda921 ljvmiranda921 mentioned this pull request Jan 29, 2019
ljvmiranda921 pushed a commit that referenced this pull request Jan 29, 2019
Resolves #228 

Added a cost function decorator which allows the user to write a cost
function that serves as a model for how the cost will be computed for
every one particle. The decorator is tested with a pytest file where the
shape and the equality with an example function is checked. I added a
note to the documentation that some numpy functions will return arrays
with single values in them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation improvements or fixes enhancement Feature requests unit tests Test-related changes v.1.1.0 In pipeline for next version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants