ammarhakim / gkyl

This is the main source repo for the Gkeyll 2.0 code. Please see gkeyll.rtfd.io for details.
https://gkeyll.readthedocs.io/en/latest/
62 stars 18 forks source link

Milestones for renaming g0-merge to main #134

Open ammarhakim opened 1 year ago

ammarhakim commented 1 year ago

We are nearly ready for renaming g0-merge to main and using that for all simulations. However, we are not yet there yet. Here is a list of milestone we should meet before we do this rename. Please comment/discuss.

A secondary thing we can do but perhaps can wait is:

manauref commented 1 year ago

@ammarhakim some responses to your proposals:

  1. We might want to keep some C++ kernels in g2, for two reasons: a) there's no pressure to move them to g0 now (e.g. HasegawaWakatani), b) To leave a trace of of how g2 C++ code can be added (for dev perhaps).
  2. Clean up usage of ZeroArray: sure, easy, I think we only check for GPUs in a couple of places.
  3. Clean up CartField: I've done a lot of that already. The vast majority of what's there we still use, but there may be a couple things we can remove still.
  4. LuaJIT: you are better suited for that.
  5. Dependencies: Aside from Eigen, I don't know what else to remove. Eigen: only one place in twist-shift uses it, and we can easily remove that.
  6. Reg tests are running. Not all of course, but the key ones (ESGK and Vlasov).
manauref commented 1 year ago

I'd like to add 2 to-do's:

  1. Remove the dependency on MG Poisson solver code. There's an ES energy function in there that we are using in VP and GK.
  2. Some volume integrals (CartFieldIntegratedQuantCalc and calculation of ES energy) still have MPI inside of them. Abstract that out to Messenger instead.
  3. CartField's reduce method also has MPI inside of it. We should farm that portion out to Messenger
ammarhakim commented 1 year ago

As we discuss the merge, I would like to add the following two milestones. Once this initial discussion is complete we will make a proper list so we know what needs to be done for the merge.

manauref commented 1 year ago

We can do 1, but 2 is not needed for the merge. No one is checking regression tests in g2/master, so that is no better than what is happening in g2/g0-merge and is likely worse, because we've been working with g0-merge reg tests while master's have been neglected.

On Sun, 22 Oct 2023 at 15:44, Ammar @.***> wrote:

As we discuss the merge, I would like to add the following two milestones. Once this initial discussion is complete we will make a proper list so we know what needs to be done for the merge.

  • Add a install script to mkdeps in G2 to build GkeyllZero.
  • Get the regression system running again and look at the regression tests (work should be split between people that can properly certify sims). We should look into hooking the regression test on some systems to run semi-automatically.

— Reply to this email directly, view it on GitHub https://github.com/ammarhakim/gkyl/issues/134#issuecomment-1774219806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ6DKECYNZNK5UV5X34MGDYAWOUHAVCNFSM6AAAAAA426CK6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZUGIYTSOBQGY . You are receiving this because you were assigned.Message ID: @.***>

ammarhakim commented 1 year ago

Just because no one is checking things in master does not mean we should not check it going forward. Let us progress and not regress please :)

manauref commented 1 year ago

I'm saying merging is not a regress in that regard. If anything it is an improvement, even if it doesn't (yet) have the testing that you want.

On Mon, 23 Oct 2023 at 09:57, Ammar @.***> wrote:

Just because no one is checking things in master does not mean we should not check it going forward. Let us progress and not regress please :)

— Reply to this email directly, view it on GitHub https://github.com/ammarhakim/gkyl/issues/134#issuecomment-1775621874, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ6DKEO3XNNQAY4GKA6DD3YA2OX3AVCNFSM6AAAAAA426CK6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZVGYZDCOBXGQ . You are receiving this because you were assigned.Message ID: @.***>