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

Release v24.0 #578

Closed felker closed 4 months ago

felker commented 6 months ago

Do we want to wait to merge any leftover fixes from chemistry, radiation+CR, and multigrid CR big pull requests (#553, #537, and #570, respectively)? @c-white do we wait for a fix for #571 ?

felker commented 6 months ago

@swdavis, @tomidakn tells me that your group has a fix in the pipeline for a violation of the divergence conservation of mesh refinement in curvilinear coordinates? Is there a timeline on that, or should we push it to the v24.1 release?

swdavis commented 6 months ago

Yeah. I think my student (Maria Renee) planned to start a pull request today. She has forked the repo, implemented the fix, and tested it. I went over the code with her quickly yesterday and addressed a few style issues.I think there is some slight tentativeness because there are some outstanding issues. She implemented it only for 3D and there are some outstanding issues with using AMR with polar boundaries that are unresolved for spherical. There are some minor issues that she might want feedback from you and @tomidakn https://github.com/tomidakn, but I think those could be worked out quickly. But, its up to you guys — not sure your timeline.

-Shane

On Apr 17, 2024, at 11:01 AM, Kyle Gerard Felker @.***> wrote:

@swdavis https://github.com/swdavis, @tomidakn https://github.com/tomidakn tells me that your group has a fix in the pipeline for a violation of the divergence conservation of mesh refinement in curvilinear coordinates? Is there a timeline on that, or should we push it to the v24.1 release?

— Reply to this email directly, view it on GitHub https://github.com/PrincetonUniversity/athena/pull/578#issuecomment-2061478109, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIKYHHV7DELF3MTVAPSMATY52FF3AVCNFSM6AAAAABFSBFQTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRRGQ3TQMJQHE. You are receiving this because you were mentioned.

felker commented 6 months ago

ok sounds great! we can definitely hold the release and iterate on improving the PR--- just wanted to make sure that the timeline for submitting a draft wasnt indefinite

c-white commented 6 months ago

I think we definitely don't want to wait for a full fix to #571. Maybe just a warning/error if a bad state is reached. Perhaps "if this block is at a shearing boundary and any of its neighbors have a different level, error."

For the record, I have never in my career run a shearing box, so maybe I'm not the best for making design decisions here.

felker commented 6 months ago

Adding an error sounds good for now @c-white