-
Notifications
You must be signed in to change notification settings - Fork 5
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 new parameter programmic interface #25
base: main
Are you sure you want to change the base?
Conversation
|
||
cs = pcs.read(io.StringIO(pcss)) | ||
|
||
params = convert_from_config_space(cs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems much more complicated than calling https://mlopez-ibanez.github.io/irace/reference/read_pcs_file.html Why is necessary to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ConfigSpace seems to have made a new format called pcs_new https://automl.github.io/ConfigSpace/main/api/serialization.html#module-ConfigSpace.read_and_write.pcs_new. This would allow it to read that too because ConfigSpace
will handle the parsing. Also when parsing the conditions, ConfigSpace tries to do some complicated thing to join the conditions with and statement and check if they logically make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this new format the one used by the experiments with the surrogates? Why do we need the new format?
No. To be honestly, I only found out about the read_pcs_file function. But
I think it would be better to do it this way anyway because using
ConfigSpace own library can catch some subtleties that we can't.
Also, we don't need this, but other people might benefit from this.
…On Wed, Dec 21, 2022 at 9:47 PM Manuel López-Ibáñez < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tests/test_config_space.py
<#25 (comment)>
:
> +simplex_refactor {0, 4, 16, 64, 256} [0]
+simplex_tolerances_markowitz [0.0001, 0.5] [0.01]l
+mip_limits_strongcand [2, 40] [10]il
+mip_limits_strongit {0, 1, 4, 16, 64} [0]
+mip_strategy_order {yes, no} [yes]
+perturbation_constant [1e-08, 0.0001] [1e-06]l
+
+mip_strategy_order | mip_ordertype in {1, 2, 3}
+mip_limits_strongcand | mip_strategy_variableselect in {3}
+mip_limits_strongit | mip_strategy_variableselect in {3}
+perturbation_constant | simplex_perturbation_switch in {yes}
+'''
+
+cs = pcs.read(io.StringIO(pcss))
+
+params = convert_from_config_space(cs)
Is this new format the one used by the experiments with the surrogates?
Why do we need the new format?
—
Reply to this email directly, view it on GitHub
<#25 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJX6KFHA4Z762Q2A6BVFPN3WOMC7DANCNFSM6AAAAAATCDQSXE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Could you push this into a new branch in the |
Yeah, it's in the branch I cancelled the test because I know they were going to fail anyway because |
Thanks!
If you have a fork of irace, you could configure the branch to test against your fork. We would need to be careful to exclude those changes when merging or undo those changes just before merging. |
NP.
Done. Before we merge, we can just add a commit to change the test back to be the same as main. Since we are squash merging, the history should be clean. |
I accidentally pushed my own main branch to the dev branch of
iracepy
so the pull request #5 appears to be merged. I have deleted the dev branch now because it feels quite unnecessary. So I am reopening this PR to main.