[feature] wrapping cost function to return best value explored #33

VolodyaCO · 2023-01-02T14:38:35Z

Description

Closes #29

Please verify that you have completed the following steps

I have self-reviewed my code.
I have included test cases validating introduced feature/fix.
I have updated documentation.

… constrained

VolodyaCO · 2023-01-03T12:48:27Z

It looks like mypy is failing due to this: python/mypy#3060. Not sure what to do 🤷‍♂️

…l-scipy-opt

codecov-commenter · 2023-01-03T20:45:43Z

Codecov Report

Patch coverage: 90.16% and project coverage change: -0.78 ⚠️

Comparison is base (76e96c9) 94.95% compared to head (6e6a611) 94.17%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
- Coverage   94.95%   94.17%   -0.78%     
==========================================
  Files          28       29       +1     
  Lines        1011     1185     +174     
==========================================
+ Hits          960     1116     +156     
- Misses         51       69      +18

Impacted Files	Coverage Δ
...rquestra/opt/optimizers/simple_gradient_descent.py	`78.43% <62.06%> (-21.57%)`	⬇️
src/orquestra/opt/optimizers/scipy_optimizer.py	`93.33% <90.56%> (-6.67%)`	⬇️
...orquestra/opt/optimizers/tensor_train_optimizer.py	`97.97% <97.97%> (ø)`
...tra/opt/optimizers/pso/continuous_pso_optimizer.py	`83.87% <100.00%> (-0.34%)`	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

AthenaCaesura · 2023-01-13T16:41:40Z

Can we add an option for the user to go back to the old way of using the scipy optimizer? People might get confused if we don't 😅

src/orquestra/opt/optimizers/scipy_optimizer.py

AthenaCaesura · 2023-01-13T16:33:44Z

src/orquestra/opt/optimizers/scipy_optimizer.py

+class _CostFunctionWithBestValueType(type):
+    def __instancecheck__(cls, obj: object) -> bool:
+        if (
+            isinstance(obj, Callable)
+            and hasattr(obj, "best_value")
+            and hasattr(obj, "best_params")
+        ):
+            return True
+        return False


This is a nice little piece of code! But I'm afraid it might be needlessly opaque since isinstance is only used once here and just for an assert statement. metaclasses are considered "heavy machinery" so it's best not to use them when they could be replaced with a simple if statement.

If I remove this to check the new instance check, I get some linting problems 😢

Yes, but since you only use this isinstance once, it's not really worth changing the instancecheck method right?

I had to do this trick for the tensor train optimiser PR too #31. Do you want me to instead have a custom isinstanceCostFunctionWithBestValue function, or something like that?

No, it's probably fine. Not worth holding up the PR for this.

src/orquestra/opt/optimizers/scipy_optimizer.py

tests/orquestra/opt/optimizers/scipy_optimizer_test.py

VolodyaCO · 2023-01-25T13:37:41Z

I'm using this in a GEO workflow but it seems as though it isn't working as expected. Here are the logs from the workflow:

{"timestamp": "2023-01-25 08:32:15,166", "level": "INFO", "filename": "history_selector.py:102", "message": Number of pre-selected candidates 398. The radius is 0.0390625}
{"timestamp": "2023-01-25 08:32:15,298", "level": "INFO", "filename": "base.py:769", "message": Probs sum is: 1.0000000000000022}
{"timestamp": "2023-01-25 08:32:15,298", "level": "INFO", "filename": "base.py:770", "message": xtrain size is: 7924}
{"timestamp": "2023-01-25 08:32:15,298", "level": "INFO", "filename": "base.py:771", "message": Number of observations are: 7924}
{"timestamp": "2023-01-25 08:32:15,298", "level": "INFO", "filename": "base.py:772", "message": Minimum cost is: -8.103484550314727}
Composition labels in HDBSCAN clustering:
Counter({-1: 4496, 4: 1412, 8: 838, 7: 283, 1: 197, 5: 194, 6: 161, 0: 121, 3: 113, 2: 109})
Cleared cache for LangerInCorners(m=4) in 2 dimensions and seed 0
{"timestamp": "2023-01-25 08:32:16,405", "level": "INFO", "filename": "geo_workflows.py:315", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Finished in 0.024 seconds"}}
{"timestamp": "2023-01-25 08:32:16,405", "level": "INFO", "filename": "geo_workflows.py:316", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Optimum value: -7.866315123621101"}}
{"timestamp": "2023-01-25 08:32:16,433", "level": "INFO", "filename": "local_workflows.py:62", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :::: Cleaned history from length 30 to length 1. dl = 0.1810866855724538"}}
Cleared cache for LangerInCorners(m=4) in 2 dimensions and seed 0
{"timestamp": "2023-01-25 08:32:17,202", "level": "INFO", "filename": "geo_workflows.py:315", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Finished in 0.025 seconds"}}
{"timestamp": "2023-01-25 08:32:17,203", "level": "INFO", "filename": "geo_workflows.py:316", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Optimum value: -7.8663151681677315"}}
{"timestamp": "2023-01-25 08:32:17,227", "level": "INFO", "filename": "local_workflows.py:62", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :::: Cleaned history from length 67 to length 2. dl = 0.17779190436107137"}}
Cleared cache for LangerInCorners(m=4) in 2 dimensions and seed 0
{"timestamp": "2023-01-25 08:32:17,966", "level": "INFO", "filename": "geo_workflows.py:315", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Finished in 0.024 seconds"}}
{"timestamp": "2023-01-25 08:32:17,966", "level": "INFO", "filename": "geo_workflows.py:316", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Optimum value: -7.866315173534457"}}
{"timestamp": "2023-01-25 08:32:17,992", "level": "INFO", "filename": "local_workflows.py:62", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :::: Cleaned history from length 67 to length 1. dl = 0.17560887003556275"}}
Cleared cache for LangerInCorners(m=4) in 2 dimensions and seed 0
{"timestamp": "2023-01-25 08:32:18,650", "level": "INFO", "filename": "geo_workflows.py:315", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Finished in 0.024 seconds"}}
{"timestamp": "2023-01-25 08:32:18,651", "level": "INFO", "filename": "geo_workflows.py:316", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Optimum value: -7.856305814156559"}}
{"timestamp": "2023-01-25 08:32:18,680", "level": "INFO", "filename": "local_workflows.py:62", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :::: Cleaned history from length 62 to length 2. dl = 0.17537807662467564"}}
Cleared cache for LangerInCorners(m=4) in 2 dimensions and seed 0
{"timestamp": "2023-01-25 08:32:19,406", "level": "INFO", "filename": "geo_workflows.py:315", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Finished in 0.024 seconds"}}
{"timestamp": "2023-01-25 08:32:19,407", "level": "INFO", "filename": "geo_workflows.py:316", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :: Optimum value: -7.856305810418254"}}
{"timestamp": "2023-01-25 08:32:19,431", "level": "INFO", "filename": "local_workflows.py:62", "message": {"run_id": "wf.local_run.0000000@tsk.task_local-0.00000", "logs": "     :::: Cleaned history from length 60 to length 2. dl = 0.17537775444008236"}}

From GEO optimisation we get a minimum of -8.something. The point corresponding to this value in the cost function is set as the initial parameter of one of the local optimisation runs, which return as best value -7.8something. This is not the expected behaviour.

AthenaCaesura · 2023-03-15T14:37:54Z

Let me know if you need any help with debugging and GEO to get the behavior you're expecting! Although I'm not sure how much help I'll be!

… constrained

…' of https://github.com/zapatacomputing/orquestra-opt into fix/#29/wrap-cost-function-to-return-best-val-scipy-opt

[feature] wrapping cost function to return best value explored

8d8cf99

VolodyaCO added bug Something isn't working enhancement New feature or request labels Jan 2, 2023

VolodyaCO requested a review from mstechly January 2, 2023 14:38

VolodyaCO self-assigned this Jan 2, 2023

VolodyaCO added 2 commits January 2, 2023 11:28

[fix] complying with minimize signature

ff18230

[feature] saving best value and params only if params are bounded and…

4ce8bd7

… constrained

VolodyaCO marked this pull request as ready for review January 3, 2023 12:46

VolodyaCO added 6 commits January 3, 2023 07:48

Merge branch 'main' into fix/#29/wrap-cost-function-to-return-best-va…

0a5ce54

…l-scipy-opt

[fix] correct way of checking bounds

29fa0a3

[test] testing edge case of the langermann function

03e016f

[fix] fixes test

0bb0216

[fix] inequality constraint fails if function is less than 0 only

a7bfb2b

[style] length of comment

7026892

VolodyaCO requested review from AthenaCaesura and removed request for mstechly January 13, 2023 14:18

AthenaCaesura reviewed Jan 13, 2023

View reviewed changes

VolodyaCO added 2 commits January 16, 2023 08:24

[feature] expose argument to control if best parameters are stored

c2311d0

[feature] typing

3c84347

VolodyaCO added 6 commits May 2, 2023 07:51

[feature] wrapping cost function to return best value explored

fa7da9b

[fix] complying with minimize signature

718322c

[feature] saving best value and params only if params are bounded and…

b015210

… constrained

[fix] correct way of checking bounds

f0afb09

[test] testing edge case of the langermann function

a4ff2a7

[fix] fixes test

f17b02d

VolodyaCO added 4 commits May 2, 2023 07:51

[fix] inequality constraint fails if function is less than 0 only

de20411

[style] length of comment

e46d423

[feature] expose argument to control if best parameters are stored

c878851

[feature] typing

9818a7a

VolodyaCO force-pushed the fix/#29/wrap-cost-function-to-return-best-val-scipy-opt branch from 3c84347 to 9818a7a Compare May 2, 2023 12:51

AthenaCaesura approved these changes Jun 2, 2023

View reviewed changes

VolodyaCO added 4 commits June 2, 2023 12:18

[test] asserting that the opt value is the minimum of the history

3ee4bad

Merge branch 'fix/#29/wrap-cost-function-to-return-best-val-scipy-opt…

3905209

…' of https://github.com/zapatacomputing/orquestra-opt into fix/#29/wrap-cost-function-to-return-best-val-scipy-opt

[style] mypy corrections

8192721

[fix] seeding ttopt

6e6a611

VolodyaCO merged commit 46cd653 into main Jun 2, 2023

VolodyaCO deleted the fix/#29/wrap-cost-function-to-return-best-val-scipy-opt branch June 2, 2023 21:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] wrapping cost function to return best value explored #33

[feature] wrapping cost function to return best value explored #33

VolodyaCO commented Jan 2, 2023 •

edited

Loading

VolodyaCO commented Jan 3, 2023

codecov-commenter commented Jan 3, 2023 •

edited

Loading

AthenaCaesura commented Jan 13, 2023 •

edited

Loading

AthenaCaesura Jan 13, 2023

VolodyaCO Jan 16, 2023

AthenaCaesura Mar 15, 2023

VolodyaCO May 2, 2023 •

edited

Loading

AthenaCaesura Jun 2, 2023

VolodyaCO commented Jan 25, 2023

AthenaCaesura commented Mar 15, 2023 •

edited

Loading

[feature] wrapping cost function to return best value explored #33

[feature] wrapping cost function to return best value explored #33

Conversation

VolodyaCO commented Jan 2, 2023 • edited Loading

Description

Please verify that you have completed the following steps

VolodyaCO commented Jan 3, 2023

codecov-commenter commented Jan 3, 2023 • edited Loading

Codecov Report

AthenaCaesura commented Jan 13, 2023 • edited Loading

AthenaCaesura Jan 13, 2023

Choose a reason for hiding this comment

VolodyaCO Jan 16, 2023

Choose a reason for hiding this comment

AthenaCaesura Mar 15, 2023

Choose a reason for hiding this comment

VolodyaCO May 2, 2023 • edited Loading

Choose a reason for hiding this comment

AthenaCaesura Jun 2, 2023

Choose a reason for hiding this comment

VolodyaCO commented Jan 25, 2023

AthenaCaesura commented Mar 15, 2023 • edited Loading

VolodyaCO commented Jan 2, 2023 •

edited

Loading

codecov-commenter commented Jan 3, 2023 •

edited

Loading

AthenaCaesura commented Jan 13, 2023 •

edited

Loading

VolodyaCO May 2, 2023 •

edited

Loading

AthenaCaesura commented Mar 15, 2023 •

edited

Loading