Skip to content

Commit

Permalink
Address some review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
juntyr committed May 31, 2024
1 parent f5ad511 commit 0ae44e8
Show file tree
Hide file tree
Showing 41 changed files with 156 additions and 232 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ necsim-rust consists of the following crates:
- core/: `necsim-partitioning-core` declares the core partitioning traits
- monolithic/: `necsim-partitioning-monolithic` implements monolithic, i.e. non-parallel partitioning
- mpi/: `necsim-partitioning-mpi` implements the MPI-based partitioning backend
- threads/: `necsim-partitioning-threads` implements the multithreading partitioning backend
- threads/: `necsim-partitioning-threads` implements the multithreading-based partitioning backend
- rustcoalescence/: `rustcoalescence` provides the command-line interface.
- scenarios/: `rustcoalescence-scenarios` contains the glue code to put together the cogs for the built-in scenarios. It is specifically built only for reducing code duplication in rustcoalescence, not for giving a minimal example of how to construct a simulation.
- algorithms/:
Expand Down
3 changes: 2 additions & 1 deletion docs/simulate.ron
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@
* optional, default = "100ms" */
progress: (DurationString),
)
/* the simulation is divided up across multiple threads
/* the simulation is divided up across multiple threads, which
communicate using message passing and may share immutable data
* multi-threaded, single-process
* requires the `threads-partitioning` feature */
| Threads(
Expand Down
9 changes: 0 additions & 9 deletions necsim/core/src/landscape/location.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use serde::{Deserialize, Serialize};

use crate::cogs::Backup;

#[allow(clippy::module_name_repetitions)]
#[derive(
Eq, PartialEq, PartialOrd, Ord, Clone, Hash, Debug, Serialize, Deserialize, TypeLayout,
Expand All @@ -15,13 +13,6 @@ pub struct Location {
y: u32,
}

#[contract_trait]
impl Backup for Location {
unsafe fn backup_unchecked(&self) -> Self {
self.clone()
}
}

impl Location {
#[must_use]
pub const fn new(x: u32, y: u32) -> Self {
Expand Down
7 changes: 0 additions & 7 deletions necsim/core/src/lineage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,6 @@ impl<'de> Deserialize<'de> for GlobalLineageReference {
}
}

#[contract_trait]
impl Backup for GlobalLineageReference {
unsafe fn backup_unchecked(&self) -> Self {
GlobalLineageReference(self.0)
}
}

impl<M: MathsCore, H: Habitat<M>> LineageReference<M, H> for GlobalLineageReference {}

#[allow(clippy::module_name_repetitions)]
Expand Down
13 changes: 10 additions & 3 deletions necsim/impls/no-std/src/alias/packed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,22 +119,29 @@ impl<E: Copy + PartialEq> AliasMethodSamplerAtom<E> {
}
}

pub fn sample_event<M: MathsCore, G: RngCore<M>>(
alias_samplers: &[AliasMethodSamplerAtom<E>],
rng: &mut G,
) -> E {
Self::sample_event_with_cdf_limit(alias_samplers, rng, ClosedUnitF64::one())
}

#[allow(clippy::no_effect_underscore_binding)]
#[debug_requires(!alias_samplers.is_empty(), "alias_samplers is non-empty")]
#[debug_ensures(
old(alias_samplers).iter().map(|s| s.e).any(|e| e == ret),
"returns one of the weighted events"
)]
pub fn sample_event<M: MathsCore, G: RngCore<M>>(
pub fn sample_event_with_cdf_limit<M: MathsCore, G: RngCore<M>>(
alias_samplers: &[AliasMethodSamplerAtom<E>],
rng: &mut G,
factor: ClosedUnitF64,
limit: ClosedUnitF64,
) -> E {
use necsim_core::cogs::RngSampler;

#[allow(clippy::cast_precision_loss)]
let f =
rng.sample_uniform_closed_open().get() * factor.get() * (alias_samplers.len() as f64);
rng.sample_uniform_closed_open().get() * limit.get() * (alias_samplers.len() as f64);

#[allow(
clippy::cast_precision_loss,
Expand Down
4 changes: 2 additions & 2 deletions necsim/impls/no-std/src/array2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,15 +464,15 @@ impl<T, B: ArrayBackend<T>> Array2D<T, B> {
/// # fn main() -> Result<(), Error> {
/// let rows = vec![vec![1, 2, 3], vec![4, 5, 6]];
/// let array = VecArray2D::from_rows(&rows)?;
/// assert_eq!(array.into_row_major_inner(), vec![1, 2, 3, 4, 5, 6]);
/// assert_eq!(array.into_row_major_backend(), vec![1, 2, 3, 4, 5, 6]);
/// # Ok(())
/// # }
/// ```
///
/// [`Array2D`]: struct.Array2D.html
/// [row major order]: https://en.wikipedia.org/wiki/Row-_and_column-major_order
#[must_use]
pub fn into_row_major_inner(self) -> B {
pub fn into_row_major_backend(self) -> B {
self.array
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1100,11 +1100,4 @@ impl RngCore<IntrinsicsMathsCore> for DummyRng {
self.0.pop().unwrap()
}
}

#[contract_trait]
impl Backup for DummyRng {
unsafe fn backup_unchecked(&self) -> Self {
Self(self.0.clone())
}
}
// GRCOV_EXCL_STOP
Original file line number Diff line number Diff line change
Expand Up @@ -598,11 +598,4 @@ impl RngCore<IntrinsicsMathsCore> for DummyRng {
self.0.pop().unwrap()
}
}

#[contract_trait]
impl Backup for DummyRng {
unsafe fn backup_unchecked(&self) -> Self {
Self(self.0.clone())
}
}
// GRCOV_EXCL_STOP
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use core::marker::PhantomData;

use necsim_core::{
cogs::{
coalescence_sampler::CoalescenceRngSample, Backup, CoalescenceSampler, Habitat, MathsCore,
},
cogs::{coalescence_sampler::CoalescenceRngSample, CoalescenceSampler, Habitat, MathsCore},
landscape::{IndexedLocation, Location},
lineage::LineageInteraction,
};
Expand All @@ -25,9 +23,8 @@ impl<M: MathsCore, H: Habitat<M>> Default for IndependentCoalescenceSampler<M, H
}
}

#[contract_trait]
impl<M: MathsCore, H: Habitat<M>> Backup for IndependentCoalescenceSampler<M, H> {
unsafe fn backup_unchecked(&self) -> Self {
impl<M: MathsCore, H: Habitat<M>> Clone for IndependentCoalescenceSampler<M, H> {
fn clone(&self) -> Self {
Self(PhantomData::<(M, H)>)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use core::marker::PhantomData;

use necsim_core::{
cogs::{Backup, DispersalSampler, MathsCore, RngCore, RngSampler, SeparableDispersalSampler},
cogs::{DispersalSampler, MathsCore, RngCore, RngSampler, SeparableDispersalSampler},
landscape::Location,
};
use necsim_core_bond::{ClosedUnitF64, NonNegativeF64, PositiveF64};

use crate::cogs::habitat::almost_infinite::AlmostInfiniteHabitat;

#[allow(clippy::module_name_repetitions)]
#[derive(Clone, Debug)]
#[derive(Debug)]
#[cfg_attr(feature = "cuda", derive(rust_cuda::lend::LendRustToCuda))]
#[cfg_attr(feature = "cuda", cuda(free = "M", free = "G"))]
pub struct AlmostInfiniteClark2DtDispersalSampler<M: MathsCore, G: RngCore<M>> {
Expand Down Expand Up @@ -68,9 +68,8 @@ impl<M: MathsCore, G: RngCore<M>> AlmostInfiniteClark2DtDispersalSampler<M, G> {
}
}

#[contract_trait]
impl<M: MathsCore, G: RngCore<M>> Backup for AlmostInfiniteClark2DtDispersalSampler<M, G> {
unsafe fn backup_unchecked(&self) -> Self {
impl<M: MathsCore, G: RngCore<M>> Clone for AlmostInfiniteClark2DtDispersalSampler<M, G> {
fn clone(&self) -> Self {
Self {
shape_u: self.shape_u,
tail_p: self.tail_p,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use core::marker::PhantomData;

use necsim_core::{
cogs::{Backup, DispersalSampler, MathsCore, RngCore, SeparableDispersalSampler},
cogs::{DispersalSampler, MathsCore, RngCore, SeparableDispersalSampler},
landscape::Location,
};
use necsim_core_bond::{ClosedUnitF64, NonNegativeF64};

use crate::cogs::habitat::almost_infinite::AlmostInfiniteHabitat;

#[allow(clippy::module_name_repetitions)]
#[derive(Clone, Debug)]
#[derive(Debug)]
#[cfg_attr(feature = "cuda", derive(rust_cuda::lend::LendRustToCuda))]
#[cfg_attr(feature = "cuda", cuda(free = "M", free = "G"))]
pub struct AlmostInfiniteNormalDispersalSampler<M: MathsCore, G: RngCore<M>> {
Expand Down Expand Up @@ -39,9 +39,8 @@ impl<M: MathsCore, G: RngCore<M>> AlmostInfiniteNormalDispersalSampler<M, G> {
}
}

#[contract_trait]
impl<M: MathsCore, G: RngCore<M>> Backup for AlmostInfiniteNormalDispersalSampler<M, G> {
unsafe fn backup_unchecked(&self) -> Self {
impl<M: MathsCore, G: RngCore<M>> Clone for AlmostInfiniteNormalDispersalSampler<M, G> {
fn clone(&self) -> Self {
Self {
sigma: self.sigma,
self_dispersal: self.self_dispersal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use necsim_core::{
cogs::{DispersalSampler, Habitat, MathsCore, RngCore},
landscape::Location,
};
use necsim_core_bond::ClosedUnitF64;

use crate::alias::packed::AliasMethodSamplerAtom;

Expand Down Expand Up @@ -38,11 +37,8 @@ impl<M: MathsCore, H: Habitat<M>, G: RngCore<M>> DispersalSampler<M, H, G>
let alias_dispersals_at_location =
unsafe { &self.alias_dispersal_buffer.get_unchecked(alias_range) };

let dispersal_target_index: usize = AliasMethodSamplerAtom::sample_event(
alias_dispersals_at_location,
rng,
ClosedUnitF64::one(),
);
let dispersal_target_index: usize =
AliasMethodSamplerAtom::sample_event(alias_dispersals_at_location, rng);

#[allow(clippy::cast_possible_truncation)]
Location::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,15 @@ impl<M: MathsCore, H: Habitat<M>, G: RngCore<M>> DispersalSampler<M, H, G>
.unwrap_unchecked()
};

// Safe by the construction of `InMemoryPackedAliasDispersalSampler`
// Safe by the construction of `InMemoryPackedSeparableAliasDispersalSampler`
let alias_dispersals_at_location = unsafe {
&self
.alias_dispersal_buffer
.get_unchecked(alias_range.start..alias_range.end)
};

let dispersal_target_index: usize = AliasMethodSamplerAtom::sample_event(
alias_dispersals_at_location,
rng,
ClosedUnitF64::one(),
);
let dispersal_target_index: usize =
AliasMethodSamplerAtom::sample_event(alias_dispersals_at_location, rng);

#[allow(clippy::cast_possible_truncation)]
Location::new(
Expand Down Expand Up @@ -82,16 +79,16 @@ impl<M: MathsCore, H: Habitat<M>, G: RngCore<M>> SeparableDispersalSampler<M, H,
.unwrap_unchecked()
};

// Safe by the construction of `InMemoryPackedAliasDispersalSampler`
// Safe by the construction of `InMemoryPackedSeparableAliasDispersalSampler`
let alias_dispersals_at_location = unsafe {
&self
.alias_dispersal_buffer
.get_unchecked(alias_range.start..alias_range.end)
};

// Since the atoms are pre-sorted s.t. all self-dispersal is on the right,
// exclude it by providing 1-sd as the index sampling factor
let mut dispersal_target_index: usize = AliasMethodSamplerAtom::sample_event(
// we can exclude sel-dispersal by providing 1-sd as the CDF limit
let mut dispersal_target_index: usize = AliasMethodSamplerAtom::sample_event_with_cdf_limit(
alias_dispersals_at_location,
rng,
self_dispersal.self_dispersal.one_minus(),
Expand Down Expand Up @@ -121,10 +118,15 @@ impl<M: MathsCore, H: Habitat<M>, G: RngCore<M>> SeparableDispersalSampler<M, H,
location: &Location,
habitat: &H,
) -> ClosedUnitF64 {
self.self_dispersal[(
location.y().wrapping_sub(habitat.get_extent().origin().y()) as usize,
location.x().wrapping_sub(habitat.get_extent().origin().x()) as usize,
)]
.self_dispersal
// Only safe as trait precondition that `location` is inside `habitat`
unsafe {
self.self_dispersal
.get(
location.y().wrapping_sub(habitat.get_extent().origin().y()) as usize,
location.x().wrapping_sub(habitat.get_extent().origin().x()) as usize,
)
.unwrap_unchecked()
}
.self_dispersal
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ impl<M: MathsCore, H: Habitat<M>, G: RngCore<M>> InMemoryDispersalSampler<M, H,
end: range_from,
}
} else {
// total weight already contains self-dispersal
let total_weight = event_weights
.iter()
.map(|(_e, w)| *w)
.sum::<NonNegativeF64>()
+ self_dispersal_at_location;
.sum::<NonNegativeF64>();
// Safety: Normalisation limits the result to [0.0; 1.0]
let self_dispersal_probability = unsafe {
ClosedUnitF64::new_unchecked(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ impl<M: MathsCore, H: Habitat<M>, G: RngCore<M>> InMemoryDispersalSampler<M, H,
}
}

// total weight needs to extra include the self-dispersal,
// since it was excluded from event_weights earlier
let total_weight = event_weights
.iter()
.map(|(_e, w)| *w)
Expand Down
9 changes: 4 additions & 5 deletions necsim/impls/no-std/src/cogs/dispersal_sampler/non_spatial.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use core::{marker::PhantomData, num::NonZeroU64};

use necsim_core::{
cogs::{Backup, DispersalSampler, Habitat, MathsCore, RngCore, SeparableDispersalSampler},
cogs::{DispersalSampler, Habitat, MathsCore, RngCore, SeparableDispersalSampler},
landscape::Location,
};
use necsim_core_bond::ClosedUnitF64;

use crate::cogs::habitat::non_spatial::NonSpatialHabitat;

#[allow(clippy::module_name_repetitions)]
#[derive(Clone, Debug)]
#[derive(Debug)]
#[cfg_attr(feature = "cuda", derive(rust_cuda::lend::LendRustToCuda))]
#[cfg_attr(feature = "cuda", cuda(free = "M", free = "G"))]
pub struct NonSpatialDispersalSampler<M: MathsCore, G: RngCore<M>> {
Expand All @@ -25,9 +25,8 @@ impl<M: MathsCore, G: RngCore<M>> Default for NonSpatialDispersalSampler<M, G> {
}
}

#[contract_trait]
impl<M: MathsCore, G: RngCore<M>> Backup for NonSpatialDispersalSampler<M, G> {
unsafe fn backup_unchecked(&self) -> Self {
impl<M: MathsCore, G: RngCore<M>> Clone for NonSpatialDispersalSampler<M, G> {
fn clone(&self) -> Self {
Self {
marker: PhantomData::<(M, G)>,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use necsim_core::{
cogs::{Backup, DispersalSampler, Habitat, MathsCore, RngCore, SeparableDispersalSampler},
cogs::{DispersalSampler, Habitat, MathsCore, RngCore, SeparableDispersalSampler},
landscape::Location,
};
use necsim_core_bond::{ClosedUnitF64, OpenClosedUnitF64 as PositiveUnitF64};
Expand All @@ -10,7 +10,7 @@ use crate::cogs::{
};

#[allow(clippy::module_name_repetitions)]
#[derive(Clone, Debug)]
#[derive(Debug)]
#[cfg_attr(feature = "cuda", derive(rust_cuda::lend::LendRustToCuda))]
#[cfg_attr(feature = "cuda", cuda(free = "M"))]
pub struct SpatiallyImplicitDispersalSampler<M: MathsCore, G: RngCore<M>> {
Expand All @@ -32,12 +32,11 @@ impl<M: MathsCore, G: RngCore<M>> SpatiallyImplicitDispersalSampler<M, G> {
}
}

#[contract_trait]
impl<M: MathsCore, G: RngCore<M>> Backup for SpatiallyImplicitDispersalSampler<M, G> {
unsafe fn backup_unchecked(&self) -> Self {
impl<M: MathsCore, G: RngCore<M>> Clone for SpatiallyImplicitDispersalSampler<M, G> {
fn clone(&self) -> Self {
Self {
local: self.local.backup_unchecked(),
meta: self.meta.backup_unchecked(),
local: self.local.clone(),
meta: self.meta.clone(),
local_migration_probability_per_generation: self
.local_migration_probability_per_generation,
}
Expand Down
Loading

0 comments on commit 0ae44e8

Please sign in to comment.