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 performance test for LexiFlow #812

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

Anonymous-submission-repo
Copy link
Contributor

@Anonymous-submission-repo Anonymous-submission-repo commented Nov 15, 2022

Why are these changes needed?

  1. Add performance test for LexiFlow
  2. Improve the efficiency of LexiFlow

Related issue number

#767

Checks

@Anonymous-submission-repo Anonymous-submission-repo changed the title Add test for LexiFlow Add reproducibility test for LexiFlow Nov 15, 2022
@skzhang1 skzhang1 requested review from qingyun-wu and sonichi and removed request for sonichi and qingyun-wu November 15, 2022 01:38
@qingyun-wu qingyun-wu self-assigned this Nov 15, 2022
Comment on lines 367 to 368
feasible_index_filter = np.where(
k_values.take(feasible_index)
Copy link
Collaborator

Choose a reason for hiding this comment

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

k_values.take(feasible_index) is calculated twice. Use a var to calculate only once.

feasible_index = [
val for val in feasible_index if val in feasible_index_prior
]
feasible_index = np.array(feasible_index).take(feasible_index_filter)
Copy link
Collaborator

Choose a reason for hiding this comment

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

move np.array() to line 263.

@@ -374,9 +374,7 @@ def update_fbest(
]
)
)[0].tolist()
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove tolist().
use
feasible_index_filter, _ =

feasible_index = [
val for val in feasible_index if val in feasible_index_prior
]
feasible_index = np.array(feasible_index).take(feasible_index_filter)
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar as above

Comment on lines 54 to 56
assert (
analysis.best_result["currin"] <= 2.2
), "lexicographic optimization not reproducible"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the test named "reproducibility"? The assertion here is about the best currin score, not reproducibility.

@Anonymous-submission-repo Anonymous-submission-repo requested review from sonichi and removed request for qingyun-wu November 15, 2022 03:02
@@ -138,5 +158,35 @@ def evaluate_function(configuration):
print(analysis.best_result)


def test_lexiflow_BraninCurrin():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def test_lexiflow_BraninCurrin():
def test_lexiflow_performance():

@qingyun-wu qingyun-wu changed the title Add reproducibility test for LexiFlow Add performance test for LexiFlow Nov 15, 2022
@qingyun-wu qingyun-wu merged commit 5eb9927 into microsoft:main Nov 15, 2022
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.

None yet

3 participants