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

[Experimental] Enable multiobjective within optimizers #775

Merged
merged 37 commits into from
Aug 5, 2020
Merged

Conversation

jrapin
Copy link
Contributor

@jrapin jrapin commented Jul 27, 2020

Types of changes

  • Docs change / refactoring / dependency upgrade
  • 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)

Motivation and Context / Related issue

How Has This Been Tested (if it applies)

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document and completed the CLA (see CLA).
  • All tests passed, and additional code has been covered with new tests.

@jrapin jrapin self-assigned this Jul 27, 2020
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 27, 2020
@jrapin jrapin added Difficulty: High Priority: Medium Status: In Progess Type: Enhancement New feature or request and removed CLA Signed Do not delete this pull request or issue due to inactivity. labels Jul 27, 2020
@teytaud
Copy link
Contributor

teytaud commented Jul 27, 2020

...good luck, that one is super scary...



# pylint: disable=redefined-outer-name,unsubscriptable-object,unused-variable,unused-import,reimported,import-outside-toplevel
def test_doc_multiobjective() -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@teytaud below is an example of how that could work

Comment on lines +82 to +85
# TODO the following is probably not good at all:
# -> +inf if no point is strictly better (but lower if it is)
if (stored_losses <= losses).all():
distance_to_pareto = min(distance_to_pareto, min(losses - stored_losses))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@teytaud any thought on this?

# TODO the following is probably not good at all:
# -> +inf if no point is strictly better (but lower if it is)

Comment on lines +58 to +59
if (self._upper_bounds > -float("inf")).all() and (losses > self._upper_bounds).all():
return float('inf') # Avoid uniformly worst points
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@teytaud I've added this case to auto-bound:
if one point is uniformly bad compared to previous ones, do not include it
that makes the example work way better. Is it a problem though?

remotely, and aggregate locally. This is what happens in the "minimize" method of optimizers.
"""

def __init__(self, upper_bounds: tp.Optional[tp.ArrayLike] = None, auto_bound: int = 15) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the auto-bound number should probably depend on the dimension :s

@jrapin
Copy link
Contributor Author

jrapin commented Aug 3, 2020

Todo:

  • integration with Experiments
  • deal with optimizers working with param.loss -> working without removing it for now (keeping both loss and losses)
  • remove multi obj descriptor in NGO? -> not for now

Base automatically changed from loss to master August 4, 2020 14:57
@jrapin jrapin mentioned this pull request Aug 4, 2020
7 tasks
@jrapin jrapin changed the title [WIP] Start multiobjective improvement [Experimental] Enable multiobjective within optimizers Aug 4, 2020
@jrapin jrapin merged commit 21b363b into master Aug 5, 2020
@jrapin jrapin deleted the multiobj branch August 5, 2020 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants