Coulaloglou coalescence kernel breaks

robertsawko / OpenPBE

An implementation of population balance solution strategies for OpenFOAM.

10 stars 6 forks source link

Coulaloglou coalescence kernel breaks #11

Open robertsawko opened 8 years ago

robertsawko commented 8 years ago

Something is wrong when it tries to read the viscosity field.

#0  Foam::error::printStack(Foam::Ostream&) at ??:?
#1  Foam::sigSegv::sigHandler(int) at ??:?
#2  ? in "/usr/lib/libc.so.6"
#3  Foam::coalescenceKernels::CoulaloglouTavlaridesC::S(Foam::dimensioned<double> const&, Foam::dimensioned<double> const&, int) const at ??:?
#4  Foam::PBEMethods::MOC::coalescenceSourceTerm(int) at ??:?
#5  Foam::PBEMethods::MOC::classSourceTerm(int) at ??:?
#6  Foam::PBEMethods::MOC::correct() at ??:?
#7  Foam::diameterModels::PBEDiameter::correct() at ??:?
#8  Foam::twoPhaseSystem::correct() at ??:?
#9  ? at ??:?
#10  __libc_start_main in "/usr/lib/libc.so.6"
#11  ? at ??:?

robertsawko commented 8 years ago

Right, it seems we have a serious validity and performance issue when using phaseModel. Phase model retunrs only tmp<scalarField> objects from this. We are saving references to these objects but I presume they get destroyed later on and our references become hanging pointers(?).

The way to solve is to call directly

    return impl_.S(xi1.value(),
                   xi2.value(),
                   epsilonField[celli],
                   dispersedPhase_.rho()[celli],
                   dispersedPhase_.nu()()[celli],
                   phase_.U().mesh().V()[celli]);

There's a big difference between rho and nu up there. rho is already calculated and we just access specific variable. nu has to precomputed every time for the whole field only for us to pick a single value.

robertsawko commented 8 years ago

Right, so the plan is as follows:

I've added an additional function to coalescence kernel which precalculate all tmp fields nud in case of C&C
Method code calls the update function before calculating source term fields.
S function of the kernel uses internal precalculated fields.

@Kojirion , can I ask you to get your head around this problem? Please check whether we still have performance problem or if I accidentally created a memory leak. Is there a better way which would not involve an update function at all? How do we encapsulate that functionality?