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

Cbranincurrin synthetic test function for cmoo #692

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

TsingQAQ
Copy link
Collaborator

@TsingQAQ TsingQAQ commented Feb 3, 2023

This is a (draft) PR for adding a commonly utilized synthetic test function for the multi-objective constraint function, which can be used in both notebooks and tests.

  • since haven't aware of any constraint objective function class but only one for the input, might worth discussion whether having a class ConstrainedMultiObjectiveTestProblem(MultiObjectiveTestProblem): as in this PR is the ideal approach. Alternatively, one can return a concatenation of objective function value and constraint in one function.
  • follow the notebook, for constraint < 0 is feasible

@TsingQAQ TsingQAQ marked this pull request as ready for review February 3, 2023 19:29
Copy link
Collaborator

@hstojic hstojic left a comment

Choose a reason for hiding this comment

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

thanks for the proposition!
left some comments - in short I think we need to modify the interface a bit...

trieste/objectives/multi_objectives.py Outdated Show resolved Hide resolved
trieste/objectives/multi_objectives.py Outdated Show resolved Hide resolved
trieste/objectives/multi_objectives.py Show resolved Hide resolved
trieste/objectives/multi_objectives.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@hstojic hstojic left a comment

Choose a reason for hiding this comment

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

almost there, some further minor comments

"""

def gen_pareto_optimal_points(n: int, seed: int | None = None) -> TensorType:
return tf.zeros(shape=0, dtype=tf.float64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

so there is no known pareto front for this problem I take it?
can you please add a note to the docs of the problem above stating that and that this function will return an empty tensor please

Copy link
Collaborator

Choose a reason for hiding this comment

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

@uri-granta do you think this is ok or we should return NotImplementedError? for some problems pareto front will not be available

it's a bit burdensome, but what can be done is to compute offline approximate ground truth with say genetic algos with a large number of function evaluations, store a fixed number of points in the repo as a text file and then load that when the function is called

Copy link
Collaborator

Choose a reason for hiding this comment

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

Definitely clearer to raise a NotImplementedError with a message explaining that there is no known pareto front, rather than returning an empty "front".

Copy link
Collaborator

Choose a reason for hiding this comment

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

@TsingQAQ can you please raise an error here, but perhaps in the docs you can indicate to user a way to generate approximate ground truth

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure! for readability have created a new exception class NoAnalyticalParetoPointsError, please check if this is idea.

trieste/objectives/multi_objectives.py Show resolved Hide resolved
Copy link
Collaborator

@hstojic hstojic left a comment

Choose a reason for hiding this comment

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

thanks @TsingQAQ, added a comment on your question on my suggestion about typing
and I think we need @uri-granta input on interfaces

trieste/objectives/multi_objectives.py Outdated Show resolved Hide resolved
trieste/objectives/multi_objectives.py Show resolved Hide resolved
@uri-granta uri-granta self-requested a review March 13, 2023 09:48
Copy link
Collaborator

@uri-granta uri-granta left a comment

Choose a reason for hiding this comment

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

Looks fine to me!

tests/unit/objectives/test_multi_objectives.py Outdated Show resolved Hide resolved
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