QMCPACK
Parameter Filtration in LM Optimizer
Proposed changes
This is the remainder of the changes originally proposed in #4061 . This PR allows the wave function parameters optimized by the linear method to be filtered based on the amount of noise in the parameter gradients. The LM will then optimize only a subset of the parameters while the others can be handled by accelerated descent in a hybrid optimization.
The main changes are in the LM engine and the QMCFixedSampleLinearOptimizeBatched driver. There are a few additional user inputs for turning on parameter filtration, which are currently being handled similarly to the many other driver inputs. The parameter filtration should now be compatible with both MPI and OMP parallelism. Thanks again for all the feedback so far and I am always happy to make further improvements.
What type(s) of changes does this code introduce?
- New feature
Does this introduce a breaking change?
- No
What systems has this change been tested on?
Lawrencium computing cluster
Checklist
- Yes. This PR is up to date with current the current state of 'develop'
- Yes. Code added or changed in the PR has been clang-formatted
- No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
- Yes. Documentation has been added (if appropriate)
Thanks for the feedback. The new set of changes have the history arrays in the LM engine sized according to the number of samples on an MPI rank and different threads should store data in them using the index offsets. Tests I've done with multiple threads have worked, but there could be cases I've overlooked. I'm happy to do further rounds of fixes as needed.
Do you have an example input file for this feature? Ideally there should be a test added that also serves as an example. However, parameter filtering gets used when scaling up the number of parameters than can be handled and a test that is small enough to be a good test might not be a good example. It also sounds like parameter filtering gets used in combination with hybrid optimization. It would be good to have a more complete example to see how the feature works in use. Attaching files to the PR is good enough for now.
Sure, here are input files for a quick example with the carbon trimer. The actual optimization results won't be very meaningful with low sample numbers and filtering an already small number of parameters, but the example_filter_hybrid.xml file shows the new inputs in the context of a hybrid optimization. Parameter filtration can be used for the LM on its own just by using the LM section of the hybrid input as a standalone, but I would generally expect the results to be inferior to allowing descent to optimize any parameters the LM leaves alone. I think this PR should already have some changes to the documentation to explain the new inputs, but I can add more if needed.
Added a small fix to the legacy driver to correct a problem @ye-luo noticed with PR #4075. At this point, what else is needed before this PR can be merged? I'm happy to revise the handling of threading more if desired. I'm also willing to come up with tests if needed though I wanted to be sure the threading issues were settled before trying to design any.
@LeonOtis could you make a new branch from develop and cherry pick the last fix commit you pushed and then make a separate PR.
I've now added a unit test for parameter filtration. Are there any other changes I should make before this PR can be merged? @ye-luo @markdewing
@LeonOtis I will make a review of this PR within this week. Q: with the current develop, does the "hybrid" method working with batched driver? When you made the fix to the legacy driver, I noticed there were no corresponding tests for the batched driver. If the current develop already works with "hybrid" and batched driver, could you introduce some tests.
@ye-luo The hybrid method should work with the batched driver. I've now added a test for it on H4, similar to what I previously added for the legacy driver.
@ye-luo The hybrid method should work with the batched driver. I've now added a test for it on H4, similar to what I previously added for the legacy driver.
Thank you. Could you make a separate PR with the new commits to develop? Since it works with the develop already.
@ye-luo The hybrid method should work with the batched driver. I've now added a test for it on H4, similar to what I previously added for the legacy driver.
Thank you. Could you make a separate PR with the new commits to develop? Since it works with the develop already.
Ok, I've opened a new PR #4172 that just adds the test.