python-hydro / pyro2 Goto Github PK

the pyro binder launch is not working

switch mk.sh and pyro.py to default to python 3

The sphinx docs (multigrid_basics.rst) have a projection example that is missing a figure. It is meant to be the output of project-periodic.py, but that script is also missing.

There is this script in your hydro_examples repo, but I'm not sure it's doing the same thing?

There have been a lot of updates since the original pyro paper, and more importantly, more contributors, so it is a good time for a new pub, this time via JOSS.

This is unrelated to my JOSS review so don't take this as a blocker for paper acceptance.

You may want to consider making pyro a pip-installable package, rather than relying on the directory structure of the git repository to make it runnable. This will make it easier for users to install pyro (people could just do pip install pyro2 or conda install -c conda-forge pyro2) and also make it easier for developers who have worked with other python packages to get set up initially. If you did this you could make the pyro.py script an executable that gets installed to a user's UNIX PATH out of the box, so there would be no need to always work in the pyro2 directory or refer to the pyro.py executable using its full path.

Need to better document the flow of the code, including:

-- how to add custom BCs
-- how to add solvers
-- how to add new problems

This could have applications to the projection stuff

These solvers call method_compute_timestep to get the intiial timestep for the initial iterations, but this doesn't have the safety checks that the main compute_timestep provides -- should they call the main version?

Also, since the pressure is lagged as p^{n-1/2}, is there a correction needed to account for the fact that dt is not the same between the previous step as it is for the next? (sources don't seem to deal with this)

This should be nodal in one direction or both, e.g. to support MHD B fields

It seems that the well-balanced scheme doesn't work anymore. If I run:

./pyro.py compressible_rk hse inputs.hse vis.dovis=1 compressible.well_balanced=1

it goes 16 steps, and has all NaNs in the velocity in the output

at the moment, the docs live on the gh-pages branch, but must be manually pushed. We should automate the docs build

Energy conservation with body forces can be substantially improved if the mass fluxes are used, instead of the cell-centered estimates of the work. For general body forces, I have tested:

       energy_update  = 0.5*dtdy*(Flux_y.v(n=self.vars.idens) + Flux_y.jp(1,n=self.vars.idens)) * \
                         0.5*(old_yacc_src.v() + yacc_src.v())*myg.dy

        energy_update += 0.5*dtdx*(Flux_x.v(n=self.vars.idens) + Flux_x.ip(1,n=self.vars.idens)) * \
                         0.5*(old_xacc_src.v() + xacc_src.v())*myg.dx

        ener.v()[:,:] += energy_update

This is identical to the formulation in Springel 2010 (the Arepo paper).

I also switched the half-step prediction using the primitive variables:

    V_l[1:,:,vars.iu] += 0.5*dt*0.5*(xacc_src.d[0:-1,:]+xacc_src.d[1:,:])
    V_l[1:,:,vars.iv] += 0.5*dt*0.5*(yacc_src.d[0:-1,:]+yacc_src.d[1:,:])

    V_r[1:-1,:,vars.iu] += 0.5*dt*0.5*(xacc_src.d[2:,:]+xacc_src.d[1:-1,:])
    V_r[1:-1,:,vars.iv] += 0.5*dt*0.5*(yacc_src.d[2:,:]+yacc_src.d[1:-1,:])

and likewise for the y-direction. This seems to work well, though there may be a better way to do the indexing for the half-step variables.

we should have some tutorials

If you go to https://github.com/python-hydro/pyro2/releases, you'll see that a tag for the version of the code corresponding to the 2014 paper submission is somehow pinned to be prominently displayed in the releases page. Given the new paper and the several releases that have come out since the 2014 paper, you may want to consider adjusting the repository settings so that newer releases are visible without clicking a button on the page.

A simple MOL advection solver on mapped grids would be a first start

The gravity source term for compressible_fv4 is not treated as 4th order

It looks like at least the simple advection problem suggested in the readme runs fine on my Python3.7 setup without first setting the PYTHONPATH environment variable to point at the pyro2 directory. Is it still necessary to set that for some other purposes?

our travis only considers 3.6.

This will clean-up the code substantially, since I won't need to reimplement all the ufuncs.

A sample implementation of this is already done in the rk branch

I'm getting this error when trying to simulate the advection for advection_rk,fv4,weno solvers.

File "/home/administrator/Downloads/pyro2-master/mesh/patch.py", line 848, in cell_center_data_clone
new.derives = old.derives.copy()
AttributeError: 'list' object has no attribute 'copy'

I changed the "old.derives.copy()" to just "old.derives" the solver ran normally.

I'm using pyro2 in python 2.7 environment.

Right now we don't check that the coefficient array has the same # of ghost cells in the variable_coefficient_MG solver. We should create a CCData2d object for the coefficients and pass that through, like we do with the general solver.

But then we should also make sure that we check the number of ghost cells before copying.

Using reflecting boundary conditions in a (nearly) isothermal exponential atmosphere, I find that pyro does not ensure a zero mass flux at the lower boundary, whether or not I include gravity, and whether or not I reflect the gravity vector at the boundary (it should, of course, be reflected to match the boundary conditions, and I do so by adding a new gravitational acceleration state variable and setting its boundary condition to reflect y-odd).

I have tested this setup in Castro and Athena, which both produce identically zero mass flux at the reflecting boundary.

The initial conditions are prescribed by the following:

xmom.d[:,:] = 0.0
ymom.d[:,:] = 0.0
dens.d[:,:] = 0.0
grav.d[:,:] = grav_const # this will be reflected via the boundary conditions routines              

# set the density to be stratified in the y-direction                                               
myg = my_data.grid

p = myg.scratch_array()

dens.d[:,:] = dens1*numpy.exp(-myg.y2d/scale_height)
p.d[:,:] = dens.d*cs*cs/gamma

# set the energy (P = cs2*dens)                                                                     
ener.d[:,:] = p.d[:,:]/(gamma - 1.0) + \
    0.5*(xmom.d[:,:]**2 + ymom.d[:,:]**2)/dens.d[:,:]

In mesh.patch, we allocate the state data as (nvar, nx, ny), but in the compressible unsplit fluxes routines, we have it swapped, as (nx, ny, nvar) (first constructed in interface_f.states()).

This is also the ordering that Grid2d.scratch_array() does (nvar at the end).

We need to make all of this consistence.

We should attach units to quantities using the unyt library

At the moment, we run a simulation through the script pyro.py. It would be nice to be able to interact better in the Jupyter notebook. To make this easier, it would be useful to move most of the driver loop in pyro.py into a new class, Pyro, and have evolve methods that let us evolve the whole simulation, or perhaps just take a single step. This would be useful for interactive explorations in notebooks

Pardon me if this is silly,
If I place a breakpoint just before line 284 in simulation.py of compressible solver, to check the variables u, v, p, rho, e..
the arrays of v and energy are same values.

And also ivars.iu = ivars.iener = 1

We should create a simple MHD solver using constrained transport and a method of lines integration. This will need:

• Face-centered data (#63)
• a Riemann solver (#67)
• a MHD solver that inherits from compressible

plot.py complains about boundary conditions when run on an RT plotfile

pyro simply uses pickle to store the output, since this is easy to implement, but it is not portable. The test benchmarks were output with python3, and they are in its pickle format. As a result, python2 cannot unpickle them. This discusses some of the issues:

http://stackoverflow.com/questions/28218466/unpickling-a-python-2-object-with-python-3

Potential solutions are

• convert to h5py to write out a portable output file. This is probably best in the long run, but will add another dependency and require a read routine and reinitialization routine to give the features that unpickle does for us

• have 2 benchmarks directories, 2/ and 3/, storing the python2 and python3 output respectively.

there is a lot of redundancy in the lm_combustion solver -- it should subclass simulation_null like the others do

The advection_fv4 method does not implement limiting yet. Thus is only works for smooth problems.

Since we are interfacing with Fortran, then it is more natural to use ordering="F" with our NumPy arrays. We like to have "x" as the first index (row) and "y" as the second index (column).

Also, this will eliminate a lot of transposes for plotting and the pretty_printing

and when we just print an ndarray to the screen, x will vary horizontally instead of vertically.

We should move pyro to it's own organization

In patch.py, we create the main data as a NumPy array and then case it into an ArrayIndexer() when we do a get_var(). Why not make it ArrayIndexer() from the start?

there are some places where we do

import compressible.unsplitfluxes as *

e.g. We should not do this...

To help creators of new solvers/problems, we could have a directory containing files with outlines of the functions required, similar to yt's frontends/_skeleton directory

compare.py currently checks the outputs of benchmark files are exactly equal. Instead, it should check that they agree up to some tolerance.

This will help with the new Numba implementation as the results now seem to be more sensitive to the exact system setup/compiler.

Both viscous and inviscid

There is a start of such a solver as compressible_react using Strang splitting

We should add a section about this solver to the advection docs

Add this test problem from the Zalesak paper

We want a Particles class that can store and manage particles and update their positions based on the velocity on the grid.

We save by pickling, but with inhomogeneous BCs in the MG solvers, we pass a function reference into the BC object and that gets pickled. When we unpickle, it is invalid and throws an error.

Should there be a separate projection module that all the low speed solvers can use?

When we work on an ArrayIndexer object a, and do a.min(), we expect a scalar. NumPy gives a scalar, but our wrapped class returns a 0-dimensional ArrayIndexer object.

This is discussed here at bit:

numpy/numpy#5819

Again, not a blocker for accepting the pyro paper.

It would be cool to add support for pyro's output format to yt. If you ever wanted to add AMR support to pyro this would make the visualization and analysis problem much easier. You might also be able to do cool in-memory analysis and visualization stuff by passing data back and forth between pyro and yt via the python process.

back when pyro started, there was no cython. Now we should consider migrating to cython cause it is easier and can be used more easily on some platforms (e.g. Macs)

The RT problem seems to get pushed up from the boundary when run with compressible_rk. Other hydro problems are fine, so this is a gravity or HSE BC problem.

We need a vector graphics pyro logo