dmorse / pscfpp

Polymer Self-Consistent Field Theory (C++/CUDA version)
https://pscf-home.cems.umn.edu
GNU General Public License v3.0
25 stars 20 forks source link

Add inverseId parameter to Basis::Wave, fix handling of implicit waves #174

Closed benmagruder closed 3 months ago

benmagruder commented 3 months ago

This pull request contains 4 commits.

In the first commit, a new parameter inverseId is added to the Basis::Wave class to track the index of the inverse of that wave. During construction of the basis these indices are assigned, and the method Basis::isValid has been modified to take advantage of this new parameter as a quick way to find the inverse of the wave. Also, new tests were added to Basis::isValid to make sure these indices were assigned correctly. This commit contains changes to prdc/Basis.h and prdc/Basis.tpp.

In the second commit, a few unused variables were deleted in prdc/Basis.tpp and rpc/field/FieldIo.tpp. These variables were causing compiler warnings because they were not used, so I removed them.

In the third commit, I fixed the bug in FieldIo (both rpc and rpg) that causes an error for the P_-4_b_2 space group. In the method convertKGridToBasis when identifying a characteristic wave for a star, if an implicit wave is found, the inverse of that wave is used as the characteristic wave instead, since the inverse must be explicit. Previously, the index of the inverse was assumed to be known, but this turned out to be untrue for stars on the edge of the BZ. So now, we use the new inverseId member of the Wave class to ensure that we accurately identify the inverse wave. This commit also contains a new test added to Basis::isValid to make sure that, between a wave and its inverse, at least one wave is explicit.

In the fourth commit, I simply changed maxItr from 600 to 1000 in an example in rpg/tests, because this unit test was failing when ran on an A100 GPU. For some reason, it takes ~700 iterations on an A100 but only 400 on our K40 GPU.

After all four commits, I checked all unit tests (running the rpg unit tests on both the K40 and A100 GPUs) and they all pass. I also checked that I can now successfully run a calculation with the P_-4_b_2 space group that previously gave me an error, and indeed the calculation worked as desired.