search

LLNL / libROM

Model reduction library with an emphasis on large scale parallelism and linear subspace methods
https://www.librom.net
Other
201 stars 36 forks source link

Sample mesh manager bug fix #258

Closed dylan-copeland closed 9 months ago

dylan-copeland commented 9 months ago

The only real change in this PR is at line 887 of lib/mfem/SampleMesh.cpp in the master branch. Part of the verification on that line should be done only after the loop over all spaces. This is just a sanity check, and the changes in this PR should have no effect on any results. Other changes in this PR are just to clean up some unused variables, long lines, etc.

JacobLotz commented 9 months ago

I have verified that this PR fixes issue #242. Thanks @dylan-copeland !

dylan-copeland commented 9 months ago

Thanks @JacobLotz for the review. Sorry I did not see your issue. In the future, assigning me to an issue should help.

dylan-copeland commented 9 months ago

This looks good. I just wanted to note that the unused variable std::unique_ptr<CAROM::BasisGenerator> basis_generator; also appears in PRs #249 #254 and #260. I'm not sure if it is better to remove this line in each of those PRs or this one.

Good point, that some other PRs also have this unused variable that keeps being copied to new examples. Since this PR is already approved, I think other PRs will either have the variable removed when merging with master, or they can remove their instances.