Skip to content

Update dependencies #130

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

Merged
merged 2 commits into from
Apr 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ name = "lumol"
chemfiles = "0.7"
rand = "0.3"
log = "0.3"
log-once = "0.1.1"
log-once = "0.1"
special = "0.7"
bitflags = "0.7"
ndarray = "0.6"
bitflags = "0.8"
ndarray = "0.8"
lazy_static = "0.2"
num-traits = "0.1"

Expand Down
10 changes: 5 additions & 5 deletions src/core/src/sys/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl EnergyCache {

/// Clear all values in the cache by setting them to 0
fn clear(&mut self) {
self.pairs_cache.assign(0.0);
self.pairs_cache.fill(0.0);
self.pairs = 0.0;
self.pairs_tail = 0.0;
self.bonds = 0.0;
Expand Down Expand Up @@ -248,9 +248,9 @@ impl EnergyCache {
cache.coulomb += coulomb_delta;
cache.global += global_delta;

let (n, m) = new_pairs.shape();
let (n, m) = new_pairs.dim();
debug_assert_eq!(n, m);
debug_assert_eq!((n, m), cache.pairs_cache.shape());
debug_assert_eq!((n, m), cache.pairs_cache.dim());
// only loop over the indices that actually changed
for &i in &idxes {
for j in 0..n {
Expand Down Expand Up @@ -333,9 +333,9 @@ impl EnergyCache {
cache.coulomb = new_coulomb;
cache.global = new_global;

let (n, m) = new_pairs.shape();
let (n, m) = new_pairs.dim();
debug_assert_eq!(n, m);
debug_assert_eq!((n, m), cache.pairs_cache.shape());
debug_assert_eq!((n, m), cache.pairs_cache.dim());
for (i, mi) in system.molecules().iter().enumerate() {
for mj in system.molecules().iter().skip(i + 1) {
for pi in mi.iter() {
Expand Down
134 changes: 55 additions & 79 deletions src/core/src/types/arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
// Copyright (C) 2015-2016 Lumol's contributors — BSD license

//! Multi-dimensional arrays based on ndarray
use ndarray::{Array, Ix};
use ndarray;

use std::ops::{Index, IndexMut};
use std::ops::{Index, IndexMut, Deref, DerefMut};
use types::Zero;

/// Two dimensional tensors, based on ndarray.
Expand All @@ -23,21 +23,7 @@ use types::Zero;
/// assert_eq!(a[(0, 4)], 7.0);
/// ```
#[derive(Debug, Clone, PartialEq)]
pub struct Array2<T>(Array<T, (Ix, Ix)>);

impl<T> Array2<T> {
/// Get the shape of the array
/// # Examples
/// ```
/// # use lumol::types::Array2;
/// let a: Array2<f64> = Array2::zeros((3, 5));
/// assert_eq!(a.shape(), (3, 5));
/// ```
pub fn shape(&self) -> (Ix, Ix) {
let shape = self.0.shape();
(shape[0], shape[1])
}
}
pub struct Array2<T>(ndarray::Array2<T>);

impl<T: Zero + Clone> Array2<T> {
/// Create a new `Array2` of the specified `size` filled with the
Expand All @@ -50,8 +36,8 @@ impl<T: Zero + Clone> Array2<T> {
/// let a: Array2<f64> = Array2::zeros((8, 5));
/// assert_eq!(a[(6, 2)], 0.0);
/// ```
pub fn zeros(size: (Ix, Ix)) -> Array2<T> {
Array2(Array::<T, (Ix, Ix)>::zeros(size))
pub fn zeros(size: (usize, usize)) -> Array2<T> {
Array2(ndarray::Array2::zeros(size))
}

/// Resize the array if the current size is not `size`, and fill the
Expand All @@ -73,28 +59,13 @@ impl<T: Zero + Clone> Array2<T> {
/// a.resize_if_different((8, 9));
/// assert_eq!(a[(3, 3)], 0.0);
/// ```
pub fn resize_if_different(&mut self, size: (Ix, Ix)) {
if self.0.shape() != &[size.0, size.1] {
pub fn resize_if_different(&mut self, size: (usize, usize)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not really a comment on this PR, but this behavior is not what is expected given the name of the function. I do not expect a resize operation to overwrite values that are contained in the new size, I expect something in the line of a std::vector.resize() in C++

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, but I can't think of a better name right now.

Giving this function the std::vector::resize behaviour could also be a possibility, by round-triping to a Vec:

let vec = self.into_vec();
vec.resize(size.0 * size.1);
*self = Array::from_vec(vec, size);

I don't know whether we should rename this function or change the behaviour. Do you have any preference?

Copy link
Contributor

@antoinewdg antoinewdg Apr 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing with your solution is that it does not preserve the content of the array (because of the multi-dimensional aspect if the size of an axis that is not the zero axis changes, all the indexes will be offset), which is not a big deal in the current use cases but again suffers form the association with the resize for the stdlib.

In the cases where we use it, wouldn't it be simpler to just not use it ?
For example here having instead the following seems OK:

self.pairs_cache = Array2::zeros((system.size(), system.size()));

Or, if the time for initializing all the values becomes an issue, have an unsafe function create an uninitialized array, since in both use cases we do not care about the values inside the array and overwrite every element.

if self.dim() != size {
*self = Array2::zeros(size);
}
}
}

impl<T: Clone> Array2<T> {
/// Assign the given scalar to all entries in this array
/// # Examples
/// ```
/// # use lumol::types::Array2;
/// let mut a = Array2::zeros((8, 5));
/// a.assign(33.0);
///
/// assert_eq!(a[(3, 4)], 33.0);
/// ```
pub fn assign(&mut self, value: T) {
self.0.assign_scalar(&value);
}
}

impl<T: Default> Array2<T> {
/// Create a new `Array2` of the specified `size` filled with the
/// `Default::default` return value.
Expand All @@ -108,30 +79,44 @@ impl<T: Default> Array2<T> {
///
/// assert_eq!(a, b);
/// ```
pub fn default(size: (Ix, Ix)) -> Array2<T> {
Array2(Array::<T, (Ix, Ix)>::default(size))
pub fn default(size: (usize, usize)) -> Array2<T> {
Array2(ndarray::Array2::default(size))
}
}

impl<T> Index<(Ix, Ix)> for Array2<T> {
impl<T> Index<(usize, usize)> for Array2<T> {
type Output = T;
fn index(&self, index: (Ix, Ix)) -> &T {
fn index(&self, index: (usize, usize)) -> &T {
unsafe {
// ndarray does the check for us in debug builds
self.0.uget(index)
}
}
}

impl<T> IndexMut<(Ix, Ix)> for Array2<T> {
fn index_mut(&mut self, index: (Ix, Ix)) -> &mut T {
impl<T> IndexMut<(usize, usize)> for Array2<T> {
fn index_mut(&mut self, index: (usize, usize)) -> &mut T {
unsafe {
// ndarray does the check for us in debug builds
self.0.uget_mut(index)
}
}
}

impl<T> Deref for Array2<T> {
type Target = ndarray::Array2<T>;

fn deref(&self) -> &ndarray::Array2<T> {
&self.0
}
}

impl<T> DerefMut for Array2<T> {
fn deref_mut(&mut self) -> &mut ndarray::Array2<T> {
&mut self.0
}
}

/******************************************************************************/

/// Three dimensional tensors, based on ndarray
Expand All @@ -150,23 +135,9 @@ impl<T> IndexMut<(Ix, Ix)> for Array2<T> {
/// assert_eq!(a[(0, 4, 1)], 7.0);
/// ```
#[derive(Debug, Clone, PartialEq)]
pub struct Array3<T>(Array<T, (Ix, Ix, Ix)>);
pub struct Array3<T>(ndarray::Array3<T>);

impl<T> Array3<T> {
/// Get the shape of the array.
/// # Examples
/// ```
/// # use lumol::types::Array3;
/// let a: Array3<f64> = Array3::zeros((3, 5, 7));
/// assert_eq!(a.shape(), (3, 5, 7));
/// ```
pub fn shape(&self) -> (Ix, Ix, Ix) {
let shape = self.0.shape();
(shape[0], shape[1], shape[2])
}
}

impl<T: Zero + Clone> Array3<T> {
/// Create a new `Array3` of the specified `size` filled with the
/// `Zero::zero` return value.
///
Expand All @@ -177,8 +148,8 @@ impl<T: Zero + Clone> Array3<T> {
/// let a: Array3<f64> = Array3::zeros((8, 5, 2));
/// assert_eq!(a[(6, 2, 0)], 0.0);
/// ```
pub fn zeros(size: (Ix, Ix, Ix)) -> Array3<T> {
Array3(Array::<T, (Ix, Ix, Ix)>::zeros(size))
pub fn zeros(size: (usize, usize, usize)) -> Array3<T> where T: Zero + Clone {
Array3(ndarray::Array3::zeros(size))
}

/// Resize the array if the current size is not `size`, and fill the
Expand All @@ -200,21 +171,12 @@ impl<T: Zero + Clone> Array3<T> {
/// a.resize_if_different((8, 5, 6));
/// assert_eq!(a[(3, 3, 3)], 0.0);
/// ```
pub fn resize_if_different(&mut self, size: (Ix, Ix, Ix)) {
pub fn resize_if_different(&mut self, size: (usize, usize, usize)) where T: Zero + Clone {
if self.0.shape() != &[size.0, size.1, size.2] {
*self = Array3::zeros(size);
}
}
}

impl<T: Clone> Array3<T> {
/// Assign the given scalar to all entries in this array
pub fn assign(&mut self, value: T) {
self.0.assign_scalar(&value);
}
}

impl<T: Default> Array3<T> {
/// Create a new `Array3` of the specified `size` filled with the
/// `Default::default` return value.
/// `Default::default` return value.
Expand All @@ -228,30 +190,44 @@ impl<T: Default> Array3<T> {
///
/// assert_eq!(a, b);
/// ```
pub fn default(size: (Ix, Ix, Ix)) -> Array3<T>{
Array3(Array::<T, (Ix, Ix, Ix)>::default(size))
pub fn default(size: (usize, usize, usize)) -> Array3<T> where T: Default{
Array3(ndarray::Array3::default(size))
}
}

impl<T> Index<(Ix, Ix, Ix)> for Array3<T> {
impl<T> Index<(usize, usize, usize)> for Array3<T> {
type Output = T;
fn index(&self, index: (Ix, Ix, Ix)) -> &T {
fn index(&self, index: (usize, usize, usize)) -> &T {
unsafe {
// ndarray does the check for us in debug builds
self.0.uget(index)
}
}
}

impl<T> IndexMut<(Ix, Ix, Ix)> for Array3<T> {
fn index_mut(&mut self, index: (Ix, Ix, Ix)) -> &mut T {
impl<T> IndexMut<(usize, usize, usize)> for Array3<T> {
fn index_mut(&mut self, index: (usize, usize, usize)) -> &mut T {
unsafe {
// ndarray does the check for us in debug builds
self.0.uget_mut(index)
}
}
}

impl<T> Deref for Array3<T> {
type Target = ndarray::Array3<T>;

fn deref(&self) -> &ndarray::Array3<T> {
&self.0
}
}

impl<T> DerefMut for Array3<T> {
fn deref_mut(&mut self) -> &mut ndarray::Array3<T> {
&mut self.0
}
}

#[cfg(test)]
mod tests {
mod array2 {
Expand Down Expand Up @@ -283,11 +259,11 @@ mod tests {
#[test]
fn resize() {
let mut a: Array2<f64> = Array2::zeros((3, 4));
assert_eq!(a.shape(), (3, 4));
assert_eq!(a.dim(), (3, 4));
a[(1, 1)] = 42.0;

a.resize_if_different((7, 90));
assert_eq!(a.shape(), (7, 90));
assert_eq!(a.dim(), (7, 90));
assert_eq!(a[(1, 1)], 0.0);

a[(1, 1)] = 42.0;
Expand Down Expand Up @@ -355,11 +331,11 @@ mod tests {
#[test]
fn resize() {
let mut a: Array3<f64> = Array3::zeros((3, 4, 5));
assert_eq!(a.shape(), (3, 4, 5));
assert_eq!(a.dim(), (3, 4, 5));
a[(1, 1, 1)] = 42.0;

a.resize_if_different((7, 90, 8));
assert_eq!(a.shape(), (7, 90, 8));
assert_eq!(a.dim(), (7, 90, 8));
assert_eq!(a[(1, 1, 1)], 0.0);

a[(1, 1, 1)] = 42.0;
Expand Down
2 changes: 1 addition & 1 deletion src/input/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ license = "BSD-3-Clause"

[dependencies]
lumol-core = {path = "../core"}
toml = "0.2"
toml = "0.3"
log = "0.3"
chemfiles = "0.7"

Expand Down
30 changes: 11 additions & 19 deletions src/input/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use std::fmt;
use std::result;
use std::path::PathBuf;

use toml::Parser;
use chemfiles;

use lumol::units::ParseError;
Expand All @@ -19,7 +18,7 @@ pub type Result<T> = result::Result<T, Error>;
#[derive(Debug)]
pub enum Error {
/// Error in the TOML input file
TOML(String),
TOML(Box<error::Error>),
/// IO error, and associated file path
Io(io::Error, PathBuf),
/// Error while reading a trajectory file
Expand Down Expand Up @@ -63,31 +62,34 @@ impl From<ParseError> for Error {
impl fmt::Display for Error {
fn fmt(&self, fmt: &mut fmt::Formatter) -> result::Result<(), fmt::Error> {
use std::error::Error as StdError;
let message = match *self {
match *self {
Error::Io(ref err, ref path) => {
match err.kind() {
io::ErrorKind::NotFound => {
format!("can not find '{}'", path.display())
try!(write!(fmt, "can not find '{}'", path.display()))
}
io::ErrorKind::PermissionDenied => {
format!("permission to access '{}' denied", path.display())
try!(write!(fmt, "permission to access '{}' denied", path.display()))
}
_ => {
format!("error with '{}': {}", path.display(), self.description())
try!(write!(fmt, "error with '{}': {}", path.display(), self.description()))
}
}
}
_ => String::from(self.description())
Error::Trajectory(ref err) => try!(write!(fmt, "{}", err)),
Error::TOML(ref err) => try!(write!(fmt, "{}", err)),
Error::Config(ref err) => try!(write!(fmt, "{}", err)),
Error::Unit(ref err) => try!(write!(fmt, "{}", err)),
};
try!(write!(fmt, "{}", message));
Ok(())
}
}

impl error::Error for Error {
fn description(&self) -> &str {
match *self {
Error::TOML(ref err) | Error::Config(ref err) => err,
Error::Config(ref err) => err,
Error::TOML(ref err) => err.description(),
Error::Io(ref err, _) => err.description(),
Error::Trajectory(ref err) => err.description(),
Error::Unit(ref err) => err.description(),
Expand All @@ -103,13 +105,3 @@ impl error::Error for Error {
}
}
}

pub fn toml_error_to_string(parser: &Parser) -> String {
let errors = parser.errors.iter().map(|error|{
let (line, _) = parser.to_linecol(error.lo);
format!("{} at line {}", error.desc, line + 1)
}).collect::<Vec<_>>().join("\n ");

let plural = if errors.len() == 1 {""} else {"s"};
return format!("TOML parsing error{}: {}", plural, errors);
}
Loading