experimental-design / bofire

Experimental design and (multi-objective) bayesian optimization.
https://experimental-design.github.io/bofire/
BSD 3-Clause "New" or "Revised" License
188 stars 22 forks source link

Feature Scaling in DoE #358

Closed jduerholt closed 6 days ago

jduerholt commented 6 months ago

Hi @Osburg,

this is my interpretation of the scaling as discussed in https://github.com/experimental-design/bofire/pull/354 and https://github.com/experimental-design/bofire/issues/343.

What do you think? No testing etc. yet.

Feel also free, to proceed just from this PR, of course only if you want ;)

Best,

Johannes

jduerholt commented 6 months ago

I also added some tests, and not that I return as jacobian just the diagonal elements of the full matrix.

Osburg commented 5 months ago

Hi @jduerholt,

hmm, on my machine the tests are not failing... possible that they just randomly failed? <-- seems like this is the case. Apart from this: do my additions roughly look like you wanted it?

Cheers Aaron :)

Osburg commented 4 months ago

Hi @jduerholt :)

is this one ready to be merged? (don't want to approve my own changes without showing it to sb before :D)

cheers Aaron

jduerholt commented 4 months ago

Hi @Osburg,

thx for the reminder, I will have a look over the week.

Best,

Johannes

Osburg commented 1 month ago

Hey Johannes,

sry for being inactive for so long, I was very busy with university recently. I removed the TransformEnum and added validators for the feature range. Does it look somewhat similar to what you had in mind? I think the error in type checking comes from pytorch see here

Cheers Aaron :)

jduerholt commented 3 weeks ago

In the PR, also the typing issue is fixed.

jduerholt commented 3 weeks ago

So just merge main into your branch, as the PR is already accepted.

Osburg commented 1 week ago

Hey! in the current version the new Bounds type is used instead. Does it look ok to you now? :)

jduerholt commented 1 week ago

Perfect, I did not know that this was working. I will then make the two changes this evening and merge it then in. Thanks!