Closed jtlau closed 2 years ago
@dylan-copeland @kevinhkhuynh @siuwuncheung, it sounds like combine_samples.C does the similar thing to our offset capability. Let's discuss where is the best place to put this file and how to merge it to the existing architecture of libROM.
Currently, this PR has the capability of snapshot being subtracted by mean vector, but it does not have capability of adding the mean vector back when reconstructing the full state fields. This might be already covered by our offset capability? What do you think, @kevinhkhuynh @kevinhkhuynh @siuwuncheung ?
Thank you @jtlau for this nice example of how to combine snapshot files. We have similar scripts in codes like Laghos, which we typically call "merge" scripts, that basically read snapshot files along with some offset vector and compute the SVD on the combined or merged snapshots. I don't think we have an example of how to do this in libROM, so adding this example seems helpful to other users.
Regarding the "mean" vector, we often use an "offset" vector instead, which could be an initial state. It would be nice to have the option to use the mean, computed by this script, or to read in an arbitrary offset vector from file.
Thanks @dylan-copeland for the comments. I am including an option to subtract an arbitrary offset file currently. Do you know what format the offset might likely be in? An hdf5 snapshot file?
Thanks @dylan-copeland for the comments. I am including an option to subtract an arbitrary offset file currently. Do you know what format the offset might likely be in? An hdf5 snapshot file?
Yes, hdf5 can be assumed. The libROM Vector
class has the read
and write
functions for HDF files, which we use in Laghos for example.
@dylan-copeland I have included the option to subtract an offset, using the Vector
read
function. How does that look?
@dylan-copeland @kevinhkhuynh @siuwuncheung @jtlau, Can we think of combining "merge" in Laghos and "combineSnapshot" in SU2 into one script and include the script in libROM so that other physics solver can take advantage as well?
The remaining two comments (currently) from @dylan-copeland require a separate PR implementing a few smaller capabilities within libROM (a dimension read and editing of snapshots). I will start a new PR for those changes - they should not be complicated.
Finally got around the finishing this up.
The last PR I submitted created the functions getDim
and getNumSamples
, but only for snapshot matrix files. I generalized the functions for spatial and temporal bases just now (see latest commit).
@dylan-copeland, for your remaining suggestion, I briefly looked into adding the capability to modify existing snapshots, but there didn't seem to be a simple solution, so I agree that would be better for a future and separate PR.
Can you look over the recent changes? If it looks good I believe this PR is finished.
@jtlau Thanks for the recent changes. I agree that we can postpone subtracting an offset from a basis generator's snapshots to another PR. For now, it would be helpful to add a brief comment about how that should be implemented in the future, to eliminate the need for the second generator.
Also, the combine_samples.C
should be in the examples
directory, not tests
(which is for regression tests). Maybe you can make a new subdirectory examples/misc
for this file. This file is a general example, without concrete input/output that should be tested. It is not necessary, but if you wanted to make a test, this file could go in examples
, and most of the code in main
could be put into a function in the library which could be called from a test in the tests
directory. Also not necessary, you could add comments on how to generate some snapshot data from one of the other examples in libROM.
Finally, the code needs to be stylized, but the script stylize.sh
is broken. To fix it, this version can be copied and added to this PR:
https://github.com/LLNL/libROM/blob/save_dmd_csv/scripts/stylize.sh
If you are not familiar with astyle and would like me to do this and run astyle, just let me know and I will commit the formatting changes.
Thanks for the comments @dylan-copeland. I moved the function to examples
, and added its own CMakeLists section so it will be compiled even when not using MFEM. I was able to stylize the file, too.
I created a test in tests
, although it's not quite what you described, but I think it is a simple way to test the function and provide an example. What do you think?
Hello,
I've been using this file (
combine_samples.C
) to load in multiple snapshots (in my case, from parametric simulations), subtract the mean, and compute the SVD over the snapshots.This is similar but extended capability to
tests/load_samples.C
.I'm not sure the best location for this helper file / function, but it was simple to add it to
tests/
.I think it might be useful to anyone who needs to work with parametric simulations. Subtracting the mean is optional. There is more description and an example command in the file, included here:
Description: This is a file function that reads snapshot or basis files and computes the SVD over the group. Also provides the option to subtract the mean of the group before computing the SVD.
Example:
mpirun -n 2 ./combine_samples -b -m file1 file2 file3