Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many improvements to Grid and Dimension. #484

Merged
merged 6 commits into from
Dec 21, 2014

Conversation

ketch
Copy link
Member

@ketch ketch commented Dec 17, 2014

Some of the things in this PR are demonstrated in http://nbviewer.ipython.org/gist/ketch/1a7888d1fcc37209b260,
which is work in progress.

This PR implements the following changes:

Grid

  • add new print function for Grid with useful information
  • add grid.plot() method for 2D grids
  • remove grid.num_ghost since ghost cell information belongs to solver
  • mapc2p does not take Patch as an argument. Now it takes a number of arguments equal to num_dim, which are the coordinate arrays. Note that this is backward-incompatible, but only affects problems with mapped grids. Resolves Make the mapc2p function interfaces for running and for plotting identical #73.
  • clear cached values when a Grid or Dimension property is changed (so everything gets updated based on new property)
  • add _check_validity() method

Coordinate functions

  • add methods grid.p_centers_with_ghost and grid.p_edges_with_ghost to get physical edge locations.
  • add methods grid.p_center and grid.c_center to retrieve coordinates of a single cell center
  • make grid.compute_* methods private (by adding an initial underscore) since they should only be called internally.
  • correctly call grid._compute* (don't need to explicitly pass self)
  • simplify grid._compute_p_centers and grid._compute_p_edges (1D does not need to be a special case).
  • simplify grid._compute_c_centers_with_ghost
  • make cached centers and edges exist separately for each Grid
  • don't cache dimension or grid coordinate arrays with ghosts (since num_ghost is not a grid property)

Docstrings

Dimension

  • clear cached values when a Dimension property is changed (so everything gets updated based on new property)
  • remove dimension.num_ghost
  • make _centers and _edges exist separately for each Dimension
  • pass num_ghost as argument to dimension.*_with_ghost()
  • add _check_validity() method

- make grid.compute* methods private
- add methods grid.p_centers_with_ghost and grid.p_edges_with_ghost
- simplify grid._compute_p_centers and grid._compute_p_edges
- fix typos in docstrings
- simplify grid._compute_c_centers_with_ghost
- remove dimension.num_ghost
- make _centers and _edges exist separately for each Dimension
- make cached centers and edges exist separately for each Grid
- pass num_ghost as argument to dimension.*_with_ghost()
- don't cache dimension or grid coordinate arrays with ghosts
- print function for Grid with useful information
- mapc2p does not take Patch as an argument (now only takes 1 argument, the list of coordinate arrays)
- add grid.c_center and grid.p_center methods to give coordinates of a single cell center
- add grid.plot() method for 2D grids
- expand grid docstring
Also make better use of mapping function in annulus advection example.
@mandli
Copy link
Member

mandli commented Dec 17, 2014

I support all of these changes. I especially like the grid plots.

This makes it so that the expected inputs to mapc2p are the sequence
of coordinate arrays x,y,z.

Resolves clawpack#73 (more than 3 years later!)
@ketch
Copy link
Member Author

ketch commented Dec 18, 2014

I failed to note it originally, but the change to how mapc2p is called is technically backward-incompatible. OTOH, it's likely that people weren't using that interface before (our examples did not), so I'm in favor of merging it and sending a note to claw-users in case anyone happened to rely on it.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.14%) when pulling 00a607e on ketch:improve_geometry into 40a7249 on clawpack:master.

@mandli
Copy link
Member

mandli commented Dec 18, 2014

Is it worthwhile or possible to raise an exception JIC?

@ketch
Copy link
Member Author

ketch commented Dec 21, 2014

@mandli I added code to print a warning if the first argument to mapc2p is named grid. Somewhat crude, but probably effective.

@mandli mandli merged commit da4c2fd into clawpack:master Dec 21, 2014
ketch added a commit to ketch/pyclaw that referenced this pull request Dec 30, 2014
ketch added a commit that referenced this pull request Dec 30, 2014
Fix bug inadvertently introduced in #484: mapc2p should always return a tuple
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants