wrap SC to unit cell

[jmmshn]

When you create supercell I think the expected behavior should be that the sites in the SC sit in the unit-cell.
Currently, the behavior is:

# Si unit cell
Full Formula (Si2)
Reduced Formula: Si
abc   :   3.866974   3.866975   3.866975
angles:  60.000006  60.000002  60.000010
Sites (2)
  #  SP        a      b      c
---  ----  -----  -----  -----
  0  Si    0.875  0.875  0.875
  1  Si    0.125  0.125  0.125
# Si [[-1,1,1], [1,-1,1], [1,1,-1]] supercell 
Full Formula (Si8)
Reduced Formula: Si
abc   :   5.468729   5.468729   5.468728
angles:  90.000014  90.000001  90.000001
Sites (8)
  #  SP        a      b      c
---  ----  -----  -----  -----
  0  Si    0.875  0.875  0.875
  1  Si    1.375  1.375  0.875
  2  Si    1.375  0.875  1.375
  3  Si    0.875  1.375  1.375
  4  Si    0.125  0.125  0.125
  5  Si    0.625  0.625  0.125
  6  Si    0.625  0.125  0.625
  7  Si    0.125  0.625  0.625

The additional flag should fix this problem

[shyuep]

This should be an option, but not forced. You can modify the kwargs of the method to provide an option called to_unit_cell.

[jmmshn]

I don't think we can add a kwarg for __mul__ and I would argue that this kind of folding is the "expect" behavior when we construct supercells.

If we really can't change it maybe we should just offer a quick way for users to fold the sites back to the UC after a Structure has been initialized? I looked but it seems this kind of folding only occurs during construction.

[shyuep]

I think Structure.sanitize is what you are looking for.

[mkhorton]

For __mul__ specifically, on a change of basis, would you not reasonably expect the fractional co-ordinates to be inside the unit cell? It's a semantic question, but I think there's a good case for it.

To put it a different way, I think there's a good case that to_unit_cell should be True by default, and that people can disable it if they wish. I can imagine situations where people might not fractional co-ordinates wrapped, but it seems like this would be the exception rather than the rule.

[shyuep]

I will accept this change for now. Personally I don't usually care about wrapping because all analysis should be independent of wrapping just for robustness. Any kind of analysis that assumes the coordinates are 0-1 are prone to hard to debug errors. None of the codes in pymatgen depends on the coordinates being within the unit cell.

[mkhorton]

Any kind of analysis that assumes the coordinates are 0-1 are prone to hard to debug errors.

Agreed. Still glad to see this merged though, thank you.