Add ability to mix batch initial conditions and internal IC generation

[CompRhys]

Motivation

See #2609

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Basic testing of the code is easy the challenge is working out what the run on implications might be, will this break people's code?

Related PRs

facebook/Ax#2938

[Balandat]

Thanks. Seems like you have some dtype test issues to fix.

Also, if you rebase on the most recent main branch you don't have to run the conda test in the CI :)

[CompRhys]

locally tests still failing for FAILED test/optim/test_optimize.py::TestOptimizeAcqf::test_optimize_acqf_runs_given_batch_initial_conditions - RuntimeError: expected scalar type Long but found Float although not really sure how i've caused that

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.98%. Comparing base (a1763a1) to head (ede691c).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2610   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         196      196           
  Lines       17377    17410   +33     
=======================================
+ Hits        17375    17408   +33     
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[facebook-github-bot]

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[saitcakmak]

Looks like the PR is out of sync with the main branch. Can you do another rebase please?

[CompRhys]

In this PR as stands I was confused about the roles of q and raw_samples in the initial_batch_generator argument.

Yes this PR doesn't achieve what I intended. This was written against raw_samples but it should be against n_restarts to match the original intent.

[CompRhys]

@saitcakmak I redid this whole thing after realizing that I had got confused and failed to implement what I originally intended by following raw_samples rather than num_repeats and then got distracted focussing on the trees rather than the forest.

[CompRhys]

Apologies for linting..

Two solutions to the homotopy test failure:

1. either change the following linked lines to the snippet testing that we prune and then generate new restarts.

   botorch/test/optim/test_homotopy.py

   Lines 237 to 246 in 5d37606

   	# First time we expect to call `prune_candidates` with 4 candidates
   	self.assertEqual(
   	prune_candidates_mock.call_args_list[0][1]["candidates"].shape,
   	torch.Size([4, 1]),
   	)
   	for i in range(1, 5): # The paths should have been pruned to just one path
   	self.assertEqual(
   	prune_candidates_mock.call_args_list[i][1]["candidates"].shape,
   	torch.Size([1, 1]),
   	)

        for i in range(5):
            # assert that the number of restarts are repopulated at each step
            self.assertEqual(
                prune_candidates_mock.call_args_list[i][1]["candidates"].shape,
                torch.Size([4, 1]),
            )
            # assert that the candidates are pruned to just one candidate
            self.assertEqual(
                prune_candidates(**prune_candidates_mock.call_args_list[i][1]).shape,
                torch.Size([1, 1]),
            )

2. set shared_optimize_acqf_kwargs["raw_samples"]=None at

   botorch/botorch/optim/optimize_homotopy.py

   Line 194 in 5d37606

   # Prune candidates

   to prevent the dimensions from the pruned restarts being filled with random designs in subsequent loops.

To get current behaviour the second option is better, but in principal despite the fact that random IC designs introduced later in the homotopy might not be able to be as readily optimized to sparsity the first option doesn't seem harmful (beyond there being no computational benefit to pruning) and may have upsides.

[saitcakmak]

There is one remaining test failure and a rebase seems to be necessary. Lgtm otherwise

[facebook-github-bot]

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[Balandat]

reset: squash into single commit

Btw @CompRhys for future reference this is not really necessary - the actual commit on the main branch will be made by our github bot (while maintaining attribution to the PR author), which automatically squashes all the changes together.

[facebook-github-bot]

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[Balandat]

couple of minor nits, o/w this is good to go. If you merge in the recent changes on master that will also fix the tutorial failure.

[facebook-github-bot]

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@saitcakmak merged this pull request in 4190f74.