search

simphony / simphony-common

The native implementation of the Simphony cuds objects
BSD 2-Clause "Simplified" License
7 stars 5 forks source link

lattice_tools module for checking lattice type #240

Closed kitchoi closed 8 years ago

kitchoi commented 8 years ago

Please take a look at this module and see if it would be appropriate to include it in simphony-common instead of simphony-mayavi

Code: https://github.com/simphony/simphony-mayavi/blob/master/simphony_mayavi/cuds/lattice_tools.py

The functions are implemented using the looser definitions of some lattices following the factory functions defined in simphony-common, e.g. cubic lattice is also a tetragonal/orthorhombic/rhombohedral/monoclinic lattice. These details are documented.

tuopuu commented 8 years ago

The code looks good, @kitchoi! From the lattice-Boltzmann perspective, it's not really essential to have it in simphony-common, but of course there might be other use-cases in SimPhoNy in future that would benefit from it. Therefore, my opinion is to add it here. However, I don't see a clear most favorite place for it to be added. There are a couple of options that come to mind:

  1. Add it into the cuds/lattice.py file, but as they are not directly needed providing abc_lattice implementation, that might not the best place.
  2. Create a new folder for these kind of tools, e.g. simphony/tools. People working with meshes might have needs for something similar: for example, calculating certain properties of a mesh. These tools could also be located in the same folder. Also, H5CUDS-file version conversion tools should be put in there as well.
kitchoi commented 8 years ago

Thanks @tuopuu! I also think that option 2 is better.

kemattil commented 8 years ago

I also like option 2.

ahashibon commented 8 years ago

This is a great set of utilities which we would also use for other cases, e.g., the crystal lattice builders. The best is to move them at your earliest convenience to simphony-common under tools or utils (choose what you like better, no difference here). I would also suggest a sub category as lattice utilities/tools, e.g. so user can do this:

from simphony.utils import lattice_tools from simphony.utils import mesh_tools from simphony.utils import crystal_lattice_tools as clt

kitchoi commented 8 years ago

Thanks @ahashibon! I have pushed a PR (#249) and it is ready for review; if it looks good then it can be merged.