PrincetonUniversity / athena

Athena++ radiation GRMHD code and adaptive mesh refinement (AMR) framework
https://www.athena-astro.app
BSD 3-Clause "New" or "Revised" License
226 stars 122 forks source link

Remove extraneous DeleteAthenaArray() calls #559

Closed felker closed 7 months ago

felker commented 8 months ago

After https://github.com/PrincetonUniversity/athena/discussions/558, I glanced at the code and noticed 166 calls to DeleteAthenaArray(), and I think they were all added in the radiation merge last year #492, except for 6x in pgen/hgb.cpp and pgen/jgg.cpp. Missed this in reviewing the pull request.

Any issue with removing all of them @yanfeij @c-white @tomidakn ?

c-white commented 8 months ago

I think that would be fine and would make the code more internally consistent.

yanfeij commented 8 months ago

Do we have a test to check memory? I do remember I had some memory issues with AMR without some of the DeleteAthenaArray functions.

On Tue, Jan 30, 2024 at 3:15 PM Chris White @.***> wrote:

I think that would be fine and would make the code more internally consistent.

— Reply to this email directly, view it on GitHub https://github.com/PrincetonUniversity/athena/issues/559#issuecomment-1917818309, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCWEJ725IOI3VEDHUV5BITYRFIHLAVCNFSM6AAAAABCRY3A5OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJXHAYTQMZQHE . You are receiving this because you were mentioned.Message ID: @.***>

-- Best,

Yan-Fei Jiang Associate Research Scientist, Flatiron Institute, 160 Fifth Avenue New York, NY 10010

felker commented 8 months ago

nothing explicitly targeting memory usage or leaks in the tst/ or CI setup, I think. At one point I thought I automated Valgrind, but I dont think it is running on our Jenkins tests on stellar right now