Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coulaloglou coalescence kernel breaks #11

Open
robertsawko opened this issue Feb 2, 2016 · 2 comments
Open

Coulaloglou coalescence kernel breaks #11

robertsawko opened this issue Feb 2, 2016 · 2 comments

Comments

@robertsawko
Copy link
Owner

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
Copy link
Owner Author

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 pushed a commit that referenced this issue Feb 2, 2016
This fixes validity problem mentioned in #11.
@robertsawko
Copy link
Owner Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant