Conversation
libmfpic/Euler.cpp
Outdated
| } | ||
|
|
||
| double evaluateMaxwellian(const mfem::Vector& primitive_state, const mfem::Vector velocity,const Species& species,const int dim){ | ||
| //TODO: Do we want a default value if sigma == 0? |
There was a problem hiding this comment.
We shouldn't be able to get T<=0 so you add an error here if you'd like. I think we'd probably fail further upstream.
There was a problem hiding this comment.
If error out when sigma<=0, several of our existing tests fail that use zero temperature. We can add a warning or perhaps return a NaN or negative probability
1eda0c6 to
3b4cac6
Compare
5ea3a74 to
36d28cf
Compare
| return out; | ||
| } | ||
|
|
||
| TEST(ParticleOperations, ErrorBetweenLowFidelityChargeAndPICChargeReducesWithParticleCountForMaxwellian) { |
There was a problem hiding this comment.
@reuterb Take a look at this unit test and the following
There was a problem hiding this comment.
This is checking that the PIC charge and the Euler charge match as N_part -> \infty? I think the test name is maybe a bit confusing in that regard. You could make this more rigorous by comparing to the expected value, using the expected error (related to the variance) as the tolerance for PIC.
| } | ||
|
|
||
|
|
||
| TEST(ParticleOperations, ErrorBetweenVarianceReducedChargeAndPICChargeReducesWithParticleCountForMaxwellian) { |
There was a problem hiding this comment.
same thing here. I think we could be more direct.
We know the VR operation is unbiased so we'd expected a value of
There was a problem hiding this comment.
I may not be fully grasping what we're trying to do here though.
There was a problem hiding this comment.
This unit test and the last are simply checking that the error between the variance-reduced/low-fidelity charge and the standard PIC charge reduces with particle count. I wanted to make sure my variance reduced charge implementation was still converging towards the same value as the original PIC implementation.
I was holding off on computing variance and expectation of the estimator in a unit test since that would require multiple trials and that started to feel more like a regression test.
There was a problem hiding this comment.
For a Maxwellian, the (1-fNR/f) factor is identical to 0, so the variance reduced charge is equal to the low fidelity charge. So this is just testing convergence of the PIC charge to the low-fidelity charge. If I use a kappa in the second unit test, that may be more meaningful.
There was a problem hiding this comment.
This unit test and the last are simply checking that the error between the variance-reduced/low-fidelity charge and the standard PIC charge reduces with particle count. I wanted to make sure my variance reduced charge implementation was still converging towards the same value as the original PIC implementation.
I was holding off on computing variance and expectation of the estimator in a unit test since that would require multiple trials and that started to feel more like a regression test.
Isn't the variance reduced implementation exact in this case then? So basically these are just checking that the standard PIC charge converges?
There was a problem hiding this comment.
Yes, the variance reduced implementation is exact, that's right. That is actually the first unit test I have in the file. If we use a kappa-distribution of particles and check that the variance-reduced charge and PIC charge still converge to the same answer, that seems like a good unit test for the new code.
Co-authored-by: reuterb <bwreute@sandia.gov>
Co-authored-by: reuterb <bwreute@sandia.gov>
Co-authored-by: reuterb <bwreute@sandia.gov>
Co-authored-by: reuterb <bwreute@sandia.gov>
Co-authored-by: reuterb <bwreute@sandia.gov>
Co-authored-by: reuterb <bwreute@sandia.gov>
…unit test; switch statement on dimension
3659a39 to
8f16690
Compare
No description provided.