Skip to content

Store VOCS in Single Location in Xopt#360

Closed
electronsandstuff wants to merge 1 commit intoxopt-org:mainfrom
electronsandstuff:pierce/single-vocs
Closed

Store VOCS in Single Location in Xopt#360
electronsandstuff wants to merge 1 commit intoxopt-org:mainfrom
electronsandstuff:pierce/single-vocs

Conversation

@electronsandstuff
Copy link
Contributor

This PR is a proposal to only use a single VOCS object in Xopt.

Currently VOCS exists in two places. It is in the top level Xopt object as well as in the Generator object it contains. This could cause consistency issues where a user or some code sets one VOCS object, but not the other and issues come up.

This PR would change it so that the VOCS object is only stored in the generator. This change is made in a way that is transparent to users by turning the variable vocs into a property and passing the data around to the correct location during serialization/deserialization. Tests confirm this works.

@nikitakuklev
Copy link
Collaborator

For those using non-default wrappers around the generators like Badger and apsopt, this shouldn't change anything. It does feel a bit icky to have VOCS be an excluded field and manually adding it into dumps, but it works...

@roussel-ryan thoughts?

@roussel-ryan
Copy link
Collaborator

I've considered this for a long time, what do you think about this @ChristopherMayes

@electronsandstuff
Copy link
Contributor Author

Yeah, and this was a random thought I had last night while changing some stuff with VOCS in NSGA2Generator. It was easier to throw in a PR and say "this is what I mean" than to try and describe in an issue. So, happy for lots of discussion.

@ChristopherMayes
Copy link
Collaborator

We thought about this a while back. The original reason to separate the VOCS from the generator was that the VOCS has useful information that is independent of the generator. The idea at the time was that the user could swap out generators if they wanted to.

@roussel-ryan
Copy link
Collaborator

At this point, I don't think it is too beneficial to have the vocs outside the generator. I'm particularly concerned by cases where the vocs object in xopt is altered in some way such that its no longer consistent with the generator or vice versa. However, given how impactful this change may be, I would propose that we incorporate it into the release of Xopt 3.0

@electronsandstuff
Copy link
Contributor Author

Another note is that some generators have state that depends on VOCS, either population or a surrogate model. This means that VOCS is a property of the generator and can't be separated out. Ie, you can't have a VOCS agnostic Bayesian optimizer.

@nikitakuklev nikitakuklev added 3.0 enhancement New feature or request labels Oct 13, 2025
@roussel-ryan
Copy link
Collaborator

@electronsandstuff if you want to move this implementation to the v3.0 branch the next few weeks would be a good time to do so

@roussel-ryan
Copy link
Collaborator

Superseded by #393

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.0 enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants