clawpack / amrclaw

AMR version of Clawpack
http://www.clawpack.org
BSD 3-Clause "New" or "Revised" License
26 stars 46 forks source link

Binary output : writing out ghost cells? #236

Open donnaaboise opened 5 years ago

donnaaboise commented 5 years ago

@mjberger requested a feature to read binary data in the Matlab graphics code. This is fairly straightforward to add, but would be even easier if the ghost cell data weren't written out.

In valout.f90 (line 172 in the 2d code), the relevant output line is

                case(3)
                    ! Note: We are writing out ghost cell data also
                    i = (iadd(num_eqn, num_cells(1) + 2 * num_ghost, &
                                       num_cells(2) + 2 * num_ghost))
                    write(out_unit + 1) alloc(iadd(1, 1, 1):i)

It is obvious why writing out ghost cell data here is easier than trying to loop over interior data only. But it wouldn't be that hard to modify the code slightly so that the write statement is in a subroutine in which the data in alloc(istart:iend) is reshaped so that one can output only the interior slice . Something like :

call write_slice(mx, my,meqn,mbc,alloc(iadd(1,1,1):i),out_unit+1)

where

subroutine write_slice(mx, my,meqn,mbc,q,unitnum)
...
double precision q(meqn,1-mbc:mx+mbc,1-mbc:my+mbc)
write(unitium) q(1:meqn,1:mx,1:my)
end

Performance shouldn't be an issue, or in any case, I wouldn't expect it to be any slower than the looping that is done for the ascii output.

Would this be a useful addition to valout.f90 (both 2d and 3d versions)? Or, if someone sees an obvious problem with the proposed code above, please comment.

mjberger commented 5 years ago

unfortunately, looping over slices if a lot slower. IT’s all the system calls. The ascii output option is horribly slow, which is why I had to go to the binary format.

How about reading it all in in binary, including ghost cells and copying it in matlab, if you don’t want it all. Doing it on the matlab side might not be as slow.

Marsha

On Dec 26, 2018, at 8:15 AM, Donna Calhoun notifications@github.com wrote:

@mjberger https://github.com/mjberger requested a feature to read binary data in the Matlab graphics code. This is fairly straightforward to add, but would be even easier if the ghost cell data weren't written out.

In valout.f90 (line 172 in the 2d code), the relevant output line is

            case(3)
                ! Note: We are writing out ghost cell data also
                i = (iadd(num_eqn, num_cells(1) + 2 * num_ghost, &
                                   num_cells(2) + 2 * num_ghost))
                write(out_unit + 1) alloc(iadd(1, 1, 1):i)

It is obvious why writing out ghost cell data here is easier than trying to loop over interior data only. But it wouldn't be that hard to modify the code slightly so that the write statement is in a subroutine in which the data in alloc(istart:iend) is reshaped so that one can output only the interior slice . Something like :

call write_slice(mx, my,meqn,mbc,alloc(iadd(1,1,1):i),out_unit+1) where

subroutine write_slice(mx, my,meqn,mbc,q,unitnum) ... double precision q(meqn,1-mbc:mx+mbc,1-mbc:my+mbc) write(unitium) q(1:meqn,1:mx,1:my) end Performance shouldn't be an issue, or in any case, I wouldn't expect it to be any slower than the looping that is done for the ascii output.

Would this be a useful addition to valout.f90 (both 2d and 3d versions)? Or, if someone sees an obvious problem with the proposed code above, please comment.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clawpack/amrclaw/issues/236, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1DC-51hOS5uavXW8wZGSHlsV9PjKQUks5u83Z1gaJpZM4Zhvfh.

donnaaboise commented 5 years ago

@mjberger Okay. I was hoping that using the F90 slicing feature would not be as slow as manually looping over i.j. It isn't that big a deal to strip out the ghost cells in Matlab.

mandli commented 5 years ago

I would imagine the compiler could inline something like that. Have you tested the slow-down by chance?

donnaaboise commented 5 years ago

I am testing it now - and it seems that there is no slow-down, and in fact, writing out fewer values is faster (e.g. 0.044s vs. 0.059s). So it does seem that the compiler is able to handle the slicing efficiently.

mjberger commented 5 years ago

I will time myself too

But you can’t change it when he python expects the whole thing

From the car, mjb

On Dec 26, 2018, at 11:39 AM, Donna Calhou notifications@github.com wrote:

I am testing it now - and it seems that there is no slow-down, and in fact, writing out fewer values is faster (e.g. 0.044s vs. 0.059s). So it does seem that the compiler is able to handle the slicing efficiently.

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

donnaaboise commented 5 years ago

I've just issued a PR to AMRClaw and with my proposed changes for the binary output. I've also issued one for Visclaw PR with the binary reader for Matlab.

Especially in 3d, not writing out the ghost cells seems to save a lot in both time writing output and saves in storage.

donnaaboise commented 5 years ago

Reading binary output in Matlab is faster than reading the ascii, but this may not speed up the 3d plotting as much as one might hope. Rather, the code slows down as the number of 3d slices increases. At two different places in the code, I do basically an all-to-all comparison with slices at different orientations (x,y,z) to mask patches and to plot lines at slice intersections. The lines can be suppressed, but the masking is needed so the plotting looks okay. These bottlenecks can probably be sped up with more efficient algorithms. Anyone want to look into this?

In the meantime, set as few slices as necessary, i.e. in setplot3.m,

% setplot3.m : To speed up code, set only one slice in each direction, rather than 11. 
xSliceCoords = [0.5];    %linspace(0,1,11);
ySliceCoords = [0.5];    %linspace(0,1,11);
zSliceCoords = [0.5];    %linspace(0,1,11);

Note : users can loop over multiple slices using the sliceloop command. See

>> help clawgraphics

for more information on Clawpack Matlab graphics commands.

rjleveque commented 5 years ago

On a related note, at one point I was experimenting with creating arrays in visclaw that included the ghost cells too when reading in the binary output, after making sure these were properly set at the time they are written out in valout (which they aren't by default). For some purposes it may be useful to actually have the ghost cell values when plotting. In particular, if you want to make plots of vorticity, which requires finite differencing the velocities and doesn't look nice near patch boundaries if you switch to 1-sided differences there and use centered differences elsewhere. Also contour plots typically have a gap near patch boundaries that could perhaps be avoided if you contoured grids with one row of ghost cells included. But making all this general with an option where the user could set the number of ghost cells to include both on output and input and when plotting would take some work...

donnaaboise commented 5 years ago

I actually think it would be fairly easy to specify how many ghost cells to output. Of course, it would require an extra parameter - something like mbc_out, but it seems that writing binary output with slicing doesn't suffer from any performance hits.

On the other hand, allowing the graphics to make use of the ghost cells is another story ...

mjberger commented 5 years ago

what about geoclaw output? Is it consistent with amrclaw?

— Marsha

On Dec 26, 2018, at 6:18 PM, Donna Calhoun notifications@github.com wrote:

I've just issued a PR https://github.com/clawpack/amrclaw/pull/237 to AMRClaw and with my proposed changes for the binary output. I've also issued one for Visclaw PR https://github.com/clawpack/visclaw/pull/235 with the binary reader for Matlab.

Especially in 3d, not writing out the ghost cells seems to save a lot in both time writing output and saves in storage.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clawpack/amrclaw/issues/236#issuecomment-450039961, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1DCzR0Qc7oUobGNjm2rBA4KtlHDfACks5u9APcgaJpZM4Zhvfh.

donnaaboise commented 5 years ago

I haven't done anything with GeoClaw output. And I don't really support GeoClaw plotting in Matlab - mostly because I am guessing nobody uses Matlab with GeoClaw. But I could do something if it were useful.