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

Extended fixes to branch cdm_06262019 #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gracebato
Copy link

Additional fixes.

@lijun99
Copy link
Contributor

lijun99 commented Jun 27, 2019

The memory leak seems to come from cdm.cc->void cdm(...), P, Q, R need to be freed.

I tested your cdm code, and the annealing process finished without any apparent errors. But I used a slightly modified framework (as in my pr branch). Please try if you encounter any problems.

-Lijun

@aivazis
Copy link
Member

aivazis commented Jul 13, 2019 via email

@lijun99
Copy link
Contributor

lijun99 commented Jul 13, 2019

P,Q,R come from changes by Grace (please check her version of cdm.cc).
-Lijun

@aivazis
Copy link
Member

aivazis commented Jul 13, 2019 via email

@lijun99
Copy link
Contributor

lijun99 commented Jul 13, 2019

In her version of cdm.cc, lines 191-193
gsl_matrix * P = gsl_matrix_alloc(locations->size1, 3);
gsl_matrix * Q = gsl_matrix_alloc(locations->size1, 3);
gsl_matrix * R = gsl_matrix_alloc(locations->size1, 3);
They need to be freed at the end of routine,
gsl_matrix_free(P);
gsl_matrix_free(Q);
gsl_matrix_free(R);
otherwise, the memory keeps increasing during run (memory leak).

After adding the lines, her code seems to run ok for me, and gets reasonable results.

-Lijun

@aivazis
Copy link
Member

aivazis commented Jul 13, 2019 via email

@lijun99
Copy link
Contributor

lijun99 commented Jul 14, 2019

Yes. It's in her pull request under which we are now commenting, not the master branch.
#14
https://github.com/gracebato/altar/tree/cdm_26062019
AlTar framework should be free from memory leaks, either cpu or gpu, as I tested many times.
The issue appears to only come from the changes in her pull request.

I guess her major changes are to use different parameterset for x/y/z components for a and omega. A discussion with her might be helpful on how to merge her changes into master branch.

-Lijun

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

Successfully merging this pull request may close these issues.

3 participants