search

AlTarFramework / altar

Other
33 stars 17 forks source link

Extended fixes to branch cdm_06262019 #14

Open gracebato opened 5 years ago

gracebato commented 5 years ago

Additional fixes.

lijun99 commented 5 years ago

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 commented 5 years ago

Lijun,

Could you please be more specific about “freeing” the P,Q,R? What change to the code do you propose?

— Michael

On Jun 27, 2019, at 9:58 PM, Lijun Zhu notifications@github.com wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/AlTarFramework/altar/pull/14?email_source=notifications&email_token=ABQIFSOO5HXL5TYL4XZE3KDP4UEVPA5CNFSM4H3XDZN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYYB5ZA#issuecomment-506470116, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQIFSO246B66CDUB3CKUCLP4UEVPANCNFSM4H3XDZNQ.

lijun99 commented 5 years ago

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

aivazis commented 5 years ago

Can you point me to the line of code that you think is wrong?

-- Michael

On Jul 13, 2019, at 7:52 PM, Lijun Zhu notifications@github.com wrote:

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

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

lijun99 commented 5 years ago

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 commented 5 years ago

i don’t have code like this in my branch. is this grace’s code? is it checked in somewhere?

-- Michael

On Jul 14, 2019, at 1:24 AM, Lijun Zhu notifications@github.com wrote:

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

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

lijun99 commented 5 years ago

Yes. It's in her pull request under which we are now commenting, not the master branch. https://github.com/AlTarFramework/altar/pull/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