libROM
libROM copied to clipboard
LLNL
BasisGenerator finish output on function call
Before this PR, BasisGenerator does not finish writing out basis or snapshot files, when endSamples or writeSnapshot is called. The files are fully written only when BasisGenerator is deleted or goes out of scope, as the writer has to be deleted. This PR deletes the writer when endSamples or writeSnapshot is called, so that the files are finished. This prevents confusion and bugs, and the only disadvantage is that endSamples or writeSnapshot cannot be called twice.
Is it still possible to write both the snapshot and basis files?
I see the need for this. In tests/smoke_static.C I created an inner scope just so the files would be closed in the destructor. And I probably knew to do that only after some confusion.
But I also see in that example that writeSnapshot() and endSamples() are both called. I don't call both in practice so it might just be this testing example.
@jtlau Thanks for watching this PR. I was planning to ask whether this PR might break something, as it makes it impossible to call endSamples twice, or to call both endSamples and writeSnapshot. This is why it is a WIP. It is helpful to know that you do not see a practical use for calling both, except in a unit test. I will test Laghos and our other examples with this PR to see if anything is broken, and of course anyone who sees an issue with this PR should comment. I can also add an error message if endSamples or writeSnapshot is called after the writer is deleted, since an error message would be more helpful than the current version that simply fails.
@dylan-copeland what is the status of this PR?
It turned out that the basis generators are more complex than I first anticipated, since they support time intervals, where new intervals are made when the maximum number of samples is taken in an interval. Because of this, I did not have time or motivation yet to come up with a plan. Maybe we should consider removing the interval feature, if no one ever uses it, and it only causes unnecessary complications.
@dylan-copeland what is the status of this PR?
It turned out that the basis generators are more complex than I first anticipated, since they support time intervals, where new intervals are made when the maximum number of samples is taken in an interval. Because of this, I did not have time or motivation yet to come up with a plan. Maybe we should consider removing the interval feature, if no one ever uses it, and it only causes unnecessary complications.
I cast my vote to remove the interval feature. I do not think we use it anywhere.
I cast my vote to remove the interval feature. I do not think we use it anywhere.
I agree. We should remove that feature first in a separate PR, and then revisit this PR, which will be greatly simplified.
@dylan-copeland The plan is to remove interval concept in basis generator and then come back to this PR again.
