ConfigSpace icon indicating copy to clipboard operation
ConfigSpace copied to clipboard

Fix small bugs related to nan values in vector passed to pdf

Open Ennosigaeon opened this issue 2 years ago • 5 comments

I tried to incorporate the new prior system with SMAC and noticed some bugs regarding NaN values introduced by conditional hyperparameters. This PR resolves all issues I have encountered with conditional hyperparameters.

If you would like to, I can also provide a minimal working example showcasing the current problems.

Ennosigaeon avatar Jun 17 '22 13:06 Ennosigaeon

Codecov Report

Base: 67.97% // Head: 67.97% // No change to project coverage :thumbsup:

Coverage data is based on head (eb021c4) compared to base (88051eb). Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #256   +/-   ##
=======================================
  Coverage   67.97%   67.97%           
=======================================
  Files          25       25           
  Lines        1786     1786           
=======================================
  Hits         1214     1214           
  Misses        572      572           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Jun 17 '22 13:06 codecov[bot]

Hi @Ennosigaeon,

Sorry for the delay! A minimal working example would be great to illuminate the needs for the change, thanks for the PR! I'll review it once I can see what it aims to fix

eddiebergman avatar Jun 23 '22 12:06 eddiebergman

Hey, I just found some time to finally rework this PR. There are now two changes, each in a separate commit and with according unit tests.

  1. It is currently not possible to call ConfigSpace.substitute_hyperparameters_in_* with either InConditions or ForbiddenRelations. The first commit resolves this issue.
  2. If you are using ConfigSpaces with conditional hyperparameters, some config values in the array representation may be nan. In the current SMAC implementation of user priors all values, including potential nans, are passed to _pdf. This commit returns a PDF of 0 for nan values.

Regarding the minimal working example: You can use the changed unit tests to provoke exceptions with the current main.

Ennosigaeon avatar Jul 23 '22 12:07 Ennosigaeon

Amazing, thanks for the update, I will check this once I'm back next week!

eddiebergman avatar Jul 27 '22 20:07 eddiebergman

Yes, your assumption is correct. An additional values field is introduced but value also exists.

Ennosigaeon avatar Aug 03 '22 08:08 Ennosigaeon

Hey @Ennosigaeon, any updates on this? It would be great if we could merge this PR as the number of merge issues is increasing.

mfeurer avatar Jan 05 '23 16:01 mfeurer

@mfeurer: Sorry, I forgot about this issue. I can take a look at in February.

Ennosigaeon avatar Jan 18 '23 17:01 Ennosigaeon

Hey, sorry to bring this up again, but we're starting to move hyperparameters into individual files in #292. It would be great if we could merge this one before to avoid extensive merge conflicts..

mfeurer avatar Jan 24 '23 11:01 mfeurer

I am on vacation this week. I will take a look at it beginning of next week. Promised ;)

Ennosigaeon avatar Jan 24 '23 15:01 Ennosigaeon

Ok, everything looks fine from my side. I will just ignore the failed tests due to The job was not started because recent account payments have failed or your spending limit needs to be increased. Please check the 'Billing & plans' section in your settings.

Please let me know if you want me to change anything else.

Ennosigaeon avatar Jan 28 '23 14:01 Ennosigaeon

Thanks a lot for these updates. Indeed, github appears to have hickups and this is not due to this PR.

mfeurer avatar Jan 30 '23 12:01 mfeurer

Thanks a lot. Github has resolved the hickups, so I just merged your contribution.

mfeurer avatar Jan 30 '23 15:01 mfeurer