ADimits commented 5 years ago

In my attempt to convert Maxim Umansky's SOLT3D code from bout-dev 3 to 4, after running all of the sources through the bout_3to4.py script, and manually fixing a few things (including e.g., MZ=2^n+1 to NZ=MC=2^n, I get the output below (running on CORI at NERSC).

Before attempting to debug further, I was wondering whether (1) this is familiar and (2) whether the BOUT, Z length comment is likely to be related to the exception.

Thanks.

. . . Option 2fluid:suppress_dc = 0 (default) Option mesh:paralleltransform = identity (data_drbm/data_1e0/BOUT.inp) Option hthe0 = 0.159155 (fluxtube_drbm.grd.nc) ****NOTE: input from BOUT, Z length needs to be divided by 1.108832e+02 Error encountered Rank 3 [Mon Mar 11 11:23:51 2019] [c0-0c0s6n1] application called MPI_Abort(comm=0x84000002, 1) - process 3 ====== Exception thrown ====== Field3D: Trying to set off-centre location on non-staggered grid Did you mean to enable staggered grids?

johnomotani commented 5 years ago

I suspect this is because we moved the StaggerGrids option from the top level to the [mesh] section. I don't think the bout_3to4.py script attempts to update input files. Somewhere near the top of your BOUT.inp file should be a line like

staggergrids = true

If you move it down under [mesh] that should fix this error.

ADimits commented 5 years ago

Hi John,

Thanks for taking a stab. Maxim’s input files all appear have

StaggerGrids = true

(i.e., commented), which I assumed implied that staggeredgrids is intended to default to be false. (if this is the boutdev v3 default.)

[I have tried to set up the build to work with Totalview, but have not really succeeded with NERSC’s compiler wrappers – but expect to eventually figure that out.]

Thanks again.

Andris

--

From: johnomotani notifications@github.com Reply-To: boutproject/BOUT-dev reply@reply.github.com Date: Monday, March 11, 2019 at 12:07 PM To: boutproject/BOUT-dev BOUT-dev@noreply.github.com Cc: Default dimits1@llnl.gov, Author author@noreply.github.com Subject: Re: [boutproject/BOUT-dev] Issue attempting to convert SOLT3D from bout-dev 3 to 4 (#1637)

I suspect this is because we moved the StaggerGrids option from the top level to the [mesh] section. I don't think the bout_3to4.py script attempts to update input files. Somewhere near the top of your BOUT.inp file should be a line like

staggergrids = true

If you move it down under [mesh] that should fix this error.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/boutproject/BOUT-dev/issues/1637#issuecomment-471678964, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACTUwkPmD1-r3xzn_xkXmlqMKj64-Pflks5vVqlWgaJpZM4bpTaz.

johnomotani commented 5 years ago

Ah, OK. Sorry, we've made quite a few changes to the staggered grids support. In BOUT++ v4, with staggergrids=false it is an error to call, e.g., setLocation(CELL_YLOW), whereas previously it would have been ignored. So either comment out the setLocation calls in the C++ physics model code, or wrap them in a conditional, e.g. instead of

Vpar.setLocation(CELL_YLOW);

put

if (mesh->StaggerGrids) {
  Vpar.setLocation(CELL_YLOW);
}

implied that staggeredgrids is intended to default to be false

Indeed, StaggerGrids defaults to false in both v3 and v4 (although that may change at some point).

ADimits commented 5 years ago

Great – thanks again. I’ll try that.

Andris

--

From: johnomotani notifications@github.com Reply-To: boutproject/BOUT-dev reply@reply.github.com Date: Monday, March 11, 2019 at 12:28 PM To: boutproject/BOUT-dev BOUT-dev@noreply.github.com Cc: Default dimits1@llnl.gov, Author author@noreply.github.com Subject: Re: [boutproject/BOUT-dev] Issue attempting to convert SOLT3D from bout-dev 3 to 4 (#1637)

Ah, OK. Sorry, we've made quite a few changes to the staggered grids support. In BOUT++ v4, with staggergrids=false it is an error to call, e.g., setLocation(CELL_YLOW), whereas previously it would have been ignored. So either comment out the setLocation calls in the C++ physics model code, or wrap them in a conditional, e.g. instead of

Vpar.setLocation(CELL_YLOW);

put

if (mesh->StaggerGrids) {

Vpar.setLocation(CELL_YLOW);

}

Indeed, StaggerGrids defaults to false in both v3 and v4 (although that may change at some point).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/boutproject/BOUT-dev/issues/1637#issuecomment-471686363, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACTUwgp4fhfqA711vttQu9Y-_yV6rNtmks5vVq43gaJpZM4bpTaz.

ADimits commented 5 years ago

With that changed, I still get issues that seem to have to do with the staggered-grid support changes:

======== . .

    Option laplace:dst = 0 (default)
    Option fft:fft_measure = 0 (default)

Error encountered Rank 2 [Mon Mar 11 12:41:05 2019] [c0-0c0s2n2] application called MPI_Abort(comm=0x84000002, 1) - process 2 Rank 1 [Mon Mar 11 12:41:05 2019] [c0-0c0s2n2] application called MPI_Abort(comm=0x84000002, 1) - process 1 ====== Exception thrown ====== Asked to interpolate, but StaggerGrids is disabled! ====== Back trace ====== -> Running RHS: PvodeSolver::rhs(0.000000e+00) on line 285 of 'pvode.cxx' -> Initialising PVODE solver on line 73 of 'pvode.cxx' ====== Exception path ====== [bt] #1 [0x4892cc] [bt] #2 [0x4b08c9] . .

Thanks again.

Andris

--

From: johnomotani notifications@github.com Reply-To: boutproject/BOUT-dev reply@reply.github.com Date: Monday, March 11, 2019 at 12:28 PM To: boutproject/BOUT-dev BOUT-dev@noreply.github.com Cc: Default dimits1@llnl.gov, Author author@noreply.github.com Subject: Re: [boutproject/BOUT-dev] Issue attempting to convert SOLT3D from bout-dev 3 to 4 (#1637)

Ah, OK. Sorry, we've made quite a few changes to the staggered grids support. In BOUT++ v4, with staggergrids=false it is an error to call, e.g., setLocation(CELL_YLOW), whereas previously it would have been ignored. So either comment out the setLocation calls in the C++ physics model code, or wrap them in a conditional, e.g. instead of

Vpar.setLocation(CELL_YLOW);

put

if (mesh->StaggerGrids) {

Vpar.setLocation(CELL_YLOW);

}

Indeed, StaggerGrids defaults to false in both v3 and v4 (although that may change at some point).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/boutproject/BOUT-dev/issues/1637#issuecomment-471686363, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACTUwgp4fhfqA711vttQu9Y-_yV6rNtmks5vVq43gaJpZM4bpTaz.

johnomotani commented 5 years ago

You need to update more calls:

If you are using any of the very old staggered grid methods like Grad_par_CtoL, they'll need replacing - I hope not so I won't go into any more details now
to remove staggered-grid functionality:
1. replace interp_to(f, CELL_*) with f
2. delete any CELL_CENTRE or CELL_YLOW arguments to Grad_par, etc.
to leave the possibility to switch staggered-grid functionality (this may actually be the more straightforward solution):
1. declare a variable CELL_LOC maybe_ylow
2. during initialization, set maybe_low
```
if (mesh->StaggerGrids) {
maybe_ylow = CELL_YLOW;
} else {
maybe_ylow = CELL_CENTRE;
}
```
3. replace all instances of CELL_YLOW with maybe_ylow
  - now all fields and operators should be at CELL_CENTRE when mesh->StaggerGrids==false, so should not throw any exceptions. interp_to(f, CELL_CENTRE), for example, will do nothing for a field f whose location is CELL_CENTRE, so it's safe to leave in.

I hope that should get your code close to working. If you want me to take a look at the code, I'd be happy to if you're willing to share it (and an input file).

johnomotani commented 5 years ago

Hi Andris, did you get this issue sorted out?

ADimits commented 5 years ago

Hi John,

Thank you for checking, and sorry for not getting back to finish this out…. It is partially sorted out. ([I am at the Sherwood Conference today, with a busy schedule. I will try to give a proper update this evening (US East Coast time).]

Andris

--

From: johnomotani notifications@github.com Reply-To: boutproject/BOUT-dev reply@reply.github.com Date: Monday, April 15, 2019 at 4:31 AM To: boutproject/BOUT-dev BOUT-dev@noreply.github.com Cc: Default dimits1@llnl.gov, Author author@noreply.github.com Subject: Re: [boutproject/BOUT-dev] Issue attempting to convert SOLT3D from bout-dev 3 to 4 (#1637)

Hi Andris, did you get this issue sorted out?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/boutproject/BOUT-dev/issues/1637#issuecomment-483214799, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACTUwqH1zBYIrYPfNvHlkbkcxOoPG1ocks5vhGLvgaJpZM4bpTaz.

johnomotani commented 5 years ago

Hi Andris,

No rush! Was only wondering if we could close this issue. Since you're still working on it I'm happy to leave it open :+1:

ADimits commented 5 years ago

Hi John (and anyone else paying attention who might know),

Sorry for the long silence. I found seemingly the same issue in the conducting-wall-mode. The source, cwm.cxx, there seems to be a pretty direct translation of the one from BOUT-dev 3.1. It (the BOUT-dev version) builds, but when attempting to run it, I get the stdout/stderror output below. The BOUT.inp file in the repo has MZ = 17, but I had changed this to MZ = 16 in my attempted runs.

I wonder if there is some additional subtlety in translating/adapting the coding in the bndry_yup_Grad_par and bndry_ydown_Grad_par functions, or something I have not noticed in changes needed in the parameter settings between BOUT versions.... The failures in SOLT3D look the same and arise in examples that invoke boundary conditions that seem to be adaptations of these functions.

Initialising Laplacian inversion routines Option laplace:async = 1 (default) Option laplace:filter = 0 (default) Option laplace:maxmode = 8 (default) Option laplace:low_mem = 0 (default) Option laplace:nonuniform = 1 (default) Option laplace:all_terms = true (data/BOUT.inp) Option laplace:global_flags = 0 (default) Option laplace:inner_boundary_flags = 0 (default) Option laplace:outer_boundary_flags = 0 (default) Option laplace:include_yguards = 0 (default) Option laplace:extra_yguards_lower = 0 (default) Option laplace:extra_yguards_upper = 0 (default) Option laplace:dst = 0 (default) Option fft:fft_measure = 0 (default) Error encountered Rank 3 [Mon Jul 1 14:37:48 2019] [c0-0c0s7n0] application called MPI_Abort(comm=0x84000002, 1) - process 3 ====== Exception thrown ====== Field3D: Operation on non-finite data at [2][2][0] ====== Back trace ====== -> Field3D(Field3D&) on line 77 of 'field3d.cxx' -> Coordinates::Grad2_par2( Field3D ) on line 757 of 'coordinates.cxx' -> Running RHS: PvodeSolver::rhs(0.000000e+00) on line 285 of 'pvode.cxx' -> Initialising PVODE solver on line 73 of 'pvode.cxx' ====== Exception path ====== [bt] #1 [0x4799e2] [bt] #2 [0x4410c1]

ADimits commented 5 years ago

I meant to say "I found seemingly the same issue in the conducting-wall-mode example."

ADimits commented 5 years ago

My working hypothesis is that the problem has to do with guard-cell filling for phi.

In

void phi_sheath_bndryconds() { LAMBDA1 = 0.0; LAMBDA2 = 1.0; //LAMBDA1 = 1.0; //LAMBDA2 = log(2.0sqrt(PIfmei));

dphi_bc_ydown = fmei*0.51*nu*(LAMBDA1*phi + LAMBDA2*te);
dphi_bc_yup = -fmei*0.51*nu*(LAMBDA1*phi + LAMBDA2*te);

if I remove the "LAMBDA1*phi + "'s, then the code runs. I have tried adding a redundant mesh->communicate(comms);

and also a

mesh->communicate(phi);

in int rhs(BoutReal t) {

but these do not help. I'll look more closely at phi and how it is being filled (or not).

johnomotani commented 5 years ago

I suspect what's happened is that conducting-wall-mode got broken when we changed the default of the laplace:include_yguards option from true to false. Including

[laplace]
...
include_yguards = true

lets cwm run.

The reasons for changing the default are:

it's not clear that it's physically meaningful to take boundary-guard-cell values of vorticity and invert them to get phi. We think it makes more sense to apply a boundary condition to phi itself.
it's inefficient to invert the laplacian in the boundary guard cells if they are not needed. I'd suggest that re-writing the sheath boundary conditions to only depend on non-boundary-guard-cell values of phi would be the best solution.

ADimits commented 5 years ago

John,

Thanks so much for identifying the issue. The reasoning for the choice makes sense.... I'll try a rewrite of the sheath BC's both in the cwm example and in SOLT3D, and let you know how it goes.

ADimits commented 5 years ago

Hello John and others,

With the change that was suggested:

[laplace] ... include_yguards = true

the cwm example still does not run for me. I must still be doing something differently from the case that ran for John. Are there any particular options in the BOUT-Dev4 build that are needed?

Running with 4 processes, here is some of the process-0 log output.

Option laplace:type = cyclic (default)

Initialising Laplacian inversion routines Option laplace:async = 1 (default) Option laplace:filter = 0 (default) Option laplace:maxmode = 8 (default) Option laplace:low_mem = 0 (default) Option laplace:nonuniform = 1 (default) Option laplace:all_terms = true (data_cwm/data_100/BOUT.inp) Option laplace:global_flags = 0 (default) Option laplace:inner_boundary_flags = 0 (default) Option laplace:outer_boundary_flags = 0 (default) Option laplace:include_yguards = true (data_cwm/data_100/BOUT.inp) Option laplace:extra_yguards_lower = 0 (default) Option laplace:extra_yguards_upper = 0 (default) Option laplace:dst = 0 (default) Option fft:fft_measure = 0 (default) Error encountered ====== Exception thrown ====== Field3D: Operation on non-finite data at [2][2][0] ====== Back trace ====== -> Field3D(Field3D&) on line 77 of 'field3d.cxx' -> Coordinates::Grad_par( Field3D ) on line 675 of 'coordinates.cxx' -> Running RHS: PvodeSolver::rhs(0.000000e+00) on line 285 of 'pvode.cxx' -> Initialising PVODE solver on line 73 of 'pvode.cxx' ====== Exception path ====== [bt] #1 [0x4892cc]

johnomotani commented 5 years ago

@ADimits, I get the same error on master. I was testing on next before.

We changed a another default: all_terms is now true by default. Then the Laplace solver uses the G1 and G3 geometry coefficients, which in master were NaN in the boundary cells. Setting all_terms = false as well as include_yguards = true should get the master version of cwm to run.

The problem with G1 and G3 has been fixed in next, so on next cwm works with all_terms left on default.

@ZedThree I'll try and fix the conducting-wall-mode example so it runs for 4.3. @ADimits @bendudson it might be useful to have some script to look at the output of the example to see if it's doing something sensible. Does anyone have one somewhere?

ADimits commented 5 years ago

John, Thanks again - that worked.

Maxim has recently written scripts for a cwm linear scan with SOLT3D that might be easily adapted. I'll take a look.

Also, consistent with the philosophy of working as much as possible with include_yguards = 0 as the default, I have looked at redoing the boundary coding. I think I know how to do it for the cwm example (but have not yet fully worked it out for the particular discretization of the sheath BC in SOLT3D). I will code this into the example, and try it - most likely once I have some version of the linear-scan script that works for it.

bendudson commented 5 years ago

The CWM example was I think used for this presentation / hands-on exercise: https://bout2013.llnl.gov/pdf/2011/talks/Friedman_lapda_afternoon.pdf There is a linear dispersion solver which I think Brett put on github somewhere, but I can't seem to find it. Perhaps Maxim knows where it is, or has a better solution.

johnomotani commented 5 years ago

@ADimits - that would be great, thank you!

@bendudson is it worth putting a link to that presentation into the example as documentation? (I guess copying in the presentation itself might have copyright issues...).

ZedThree commented 4 years ago

I think this is resolved now. At least, the conducting-wall-mode example runs.

ADimits commented 4 years ago

Sorry for leaving this hanging for so long. Yes – the conducting-wall-mode example runs.

Andris

From: Peter Hill notifications@github.com Reply-To: boutproject/BOUT-dev reply@reply.github.com Date: Monday, March 23, 2020 at 8:32 AM To: boutproject/BOUT-dev BOUT-dev@noreply.github.com Cc: Default dimits1@llnl.gov, Mention mention@noreply.github.com Subject: Re: [boutproject/BOUT-dev] Issue attempting to convert SOLT3D from bout-dev 3 to 4 (#1637)

I think this is resolved now. At least, the conducting-wall-mode example runs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/boutproject/BOUT-dev/issues/1637#issuecomment-602676776, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AASNJQQ5GAN32PYJJVWCU63RI56ITANCNFSM4G5FG2ZQ.

boutproject / BOUT-dev

Issue attempting to convert SOLT3D from bout-dev 3 to 4 #1637

StaggerGrids = true