Merge pull request #2945 from heplesser/fix-target-test-problem

Correct limit for test on target fields and remove duplicate assertions
nest · Sep 15, 2023 · 4ce5ade · 4ce5ade
2 parents 8c33673 + ddd6069
commit 4ce5ade
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 13 deletions.
diff --git a/nestkernel/target.h b/nestkernel/target.h
@@ -193,11 +193,8 @@ Target::operator=( const Target& other )
 inline Target::Target( const size_t tid, const size_t rank, const synindex syn_id, const size_t lcid )
   : remote_target_id_( 0 )
 {
-  assert( tid <= MAX_TID );   // MAX_TID is allowed since it is not used as invalid value
-  assert( rank <= MAX_RANK ); // MAX_RANK is allowed since it is not used as invalid value
-  assert( syn_id < MAX_SYN_ID );
-  assert( lcid < MAX_LCID );
-
+  // We need to call set_*() methods to properly encode values in bitfield.
+  // Validity of arguments is asserted in set_*() methods.
   set_lcid( lcid );
   set_rank( rank );
   set_tid( tid );
@@ -234,7 +231,7 @@ Target::get_rank() const
 inline void
 Target::set_tid( const size_t tid )
 {
-  assert( tid <= MAX_TID );
+  assert( tid <= MAX_TID ); // MAX_TID is allowed since it is not used as invalid value
   remote_target_id_ = ( remote_target_id_ & ( ~MASK_TID ) ) | ( static_cast< uint64_t >( tid ) << BITPOS_TID );
 }
 
@@ -247,7 +244,7 @@ Target::get_tid() const
 inline void
 Target::set_syn_id( const synindex syn_id )
 {
-  assert( syn_id <= MAX_SYN_ID );
+  assert( syn_id < MAX_SYN_ID );
   remote_target_id_ = ( remote_target_id_ & ( ~MASK_SYN_ID ) ) | ( static_cast< uint64_t >( syn_id ) << BITPOS_SYN_ID );
 }
 

diff --git a/testsuite/cpptests/test_target_fields.h b/testsuite/cpptests/test_target_fields.h
@@ -54,13 +54,15 @@ BOOST_AUTO_TEST_CASE( test_target_object_type_size )
 
 BOOST_AUTO_TEST_CASE( test_target_object_type_constructor )
 {
-  std::srand( time( nullptr ) );
+  std::srand( 1234567 );
   for ( int i = 0; i < NUM_TEST_TRIALS; ++i )
   {
+    // tid and rank can take on all values up to MAX_{TID,RANK}
+    // syn_id and lcid can only take values up to MAX_{SYN_ID,LCID}-1
     const size_t tid = std::rand() % ( MAX_TID + 1 );
     const size_t rank = std::rand() % ( MAX_RANK + 1 );
-    const synindex syn_id = std::rand() % ( MAX_SYN_ID + 1 );
-    const size_t lcid = std::rand() % ( MAX_LCID + 1 );
+    const synindex syn_id = std::rand() % MAX_SYN_ID;
+    const size_t lcid = std::rand() % MAX_LCID;
 
     Target target_id_testInit( tid, rank, syn_id, lcid );
 
@@ -74,14 +76,16 @@ BOOST_AUTO_TEST_CASE( test_target_object_type_constructor )
 
 BOOST_AUTO_TEST_CASE( test_target_object_type_set_get )
 {
-  std::srand( time( nullptr ) );
+  std::srand( 2345678 );
   Target target_id_testSetGet;
   for ( int i = 0; i < NUM_TEST_TRIALS; ++i )
   {
+    // tid and rank can take on all values up to MAX_{TID,RANK}
+    // syn_id and lcid can only take values up to MAX_{SYN_ID,LCID}-1
     const size_t tid = std::rand() % ( MAX_TID + 1 );
     const size_t rank = std::rand() % ( MAX_RANK + 1 );
-    const synindex syn_id = std::rand() % ( MAX_SYN_ID + 1 );
-    const size_t lcid = std::rand() % ( MAX_LCID + 1 );
+    const synindex syn_id = std::rand() % MAX_SYN_ID;
+    const size_t lcid = std::rand() % MAX_LCID;
 
     enum_status_target_id status_target_id = TARGET_ID_UNPROCESSED;
     if ( static_cast< bool >( std::rand() % 2 ) )