Unfortunately, no improvement. I'm unable to get the test_round_trip test to fail, and so I really don't know how to approach this. The assertion is made on a pair of 32 000 token strings (both of which have the same length in the failed test), so I really don't know on this one. Do you have any idea? Is the source code different for the tests?

I ran this branch locally and got no test failures either. I will rerun these github action tests, if they fail again, I'm willing to push this through anyways and just log that this happened as an Issue.

And no, the tests running through github that you see below are the exact same tests as you have in the source code. The only difference is the enviornment they run in (Python versions, conda, how it's installed, windows/mac/linux).

Thanks, much appreciated!

I did some digging and it seems our automated tests are pulling your latest commit on this PR, they're pulling this one. I have no idea why that is ... However I can confirm your latest commit is actually correct so I would vote to disregard these checks but try and figure out why it pulls the merge commit instead of the most recent.

@mfeurer thoughts?

Can you try one last thing, just push an empty commit

git commit --allow-empty -m "Fix?"

Sorry, was away for lunch. It's done!

No problem, thanks for that! It appears it's still pulling the old version and isn't pulling the latest commit. I'm going to merge this and finally progress on to stage 2. If the error crops up there again then we will need to somehow fix it but hopefully it dissapears.

Okay I figured out the problem and could reproduce the issue. The main problem is that your fork is a good few commits behind. You can see this here where it says that you are "10 commits behind automl:master", this also means that your branches are 10 commits behind.

To make everything up to date and to be able to reproduce the error, you must:

• Update your master branch fork through github
• Go to your coding environment and pull in the latest master.
• Merge in the newest changes from master into your branch bugfixes
• pytest -k "test_round_trip" will now produce the same error.

As for why this error occurs, ideally we should have a test that is a bit more informative as to why they are not equivalent but I will look through the changes of the 10 commits that diverge and see if anything pops up that I could point you to.

Apologies this has been so complicated. This package is in need of updating all around -_-

The failed pcs file is SparrowToRiss-cssc14.pcs which is read in, written out and then read back in and then check for equality, somewhere this is failing, I'm assuming this is to do with the pcs changes that are in the next PR?

I check through the other 2 PR's, it doesn't seem the pcs file format was fixed at any point?

Well, the PCS format was not fixed, but rather disabled for ConfigSpaces that include categoricals.

I'm not sure that's something we can really do? It basically means ConfigSpace won't allow any kind of exporting for people that use categoricals. I think we would need some solution to this by the end of this PR.

@mfeurer

The only solution I can think of is to only disable it for the new feature, essentially disabling pcs export/import when a categorical distribution is non-uniform. However this is not ideal and really the pcs file format should be updated.

Well, since every categorical has weights with this solution, we'd have to make categoricals only have weights in the case where the user specifies it. It seemed like a less consistent solution in my opinion, but I guess it can be changed if you deem it to be the best solution.

Good morning. I'm on this. The bug appears because for the json serialization of the ConfigSpace SparrowToRiss-cssc14.pcs we loose some precision in the hyperparameter init-act:

From PCS: (0.14285714285714285, 0.14285714285714285, 0.14285714285714285, 0.14285714285714285, 0.14285714285714285, 0.14285714285714285, 0.14285714285714285)
From JSON: (0.14285714285714288, 0.14285714285714288, 0.14285714285714288, 0.14285714285714288, 0.14285714285714288, 0.14285714285714288, 0.14285714285714288)

Now the question is: what's going on here? I'll keep you posted.

And the issue is

The hyperparameter is serialized to json, then de-serialized from json. Until then everything is fine. Now when the constructor is called it invokes the function _get_probabilities, which in turn tries to normalize the probabilities, creating this numeric instability. The actual issue might be the fact that we renormalize the probabilities, or that the probabilities don't sum up to 1 in the first place.

Potentially we need to store and serialize the weights, and have the probabilities as an internal representation only. Then the probabilities will only be computed on __init__ from the original weights, and should always be equal. What do you think?

If you're asking me, I think that seems like a no-brainer. I don't really see why the probabilities need to be stored externally if the weights are already there.

If you're asking me, I think that seems like a no-brainer. I don't really see why the probabilities need to be stored externally if the weights are already there.

Great :) Could you please go ahead and store and serialize the weights?

Sure thing!

To simplify testing you can temporarily prune the file SparrowToRiss-cssc14.pcs to only contain:

init-act  {0,1,2,3,4,5,6}[1]       # initialize activities (0=none,1=inc-lin,2=inc-geo,3=dec-lin,4=dec-geo,5=rnd,6=abs(jw))
init-pol  {0,1,2,3,4,5}[2]       # initialize polarity (0=none,1=JW-pol,2=JW-neg,3=MOMS,4=MOMS-neg,5=rnd)

This will continue to fail as long as the bug is there.

Okay, I guess we're getting closer - it seems to me that the final test passed, right? With the remaining failing tests, and PCS files, I would like some input as to what is actually the issue, and what to do with the categorical HPs in the PCS format. Opinions?