search

DARMA-tasking / LB-analysis-framework

Analysis framework for exploring, testing, and comparing load balancing strategies
Other
3 stars 1 forks source link

#461: Improve coverage #462

Closed cwschilly closed 8 months ago

cwschilly commented 10 months ago

Fixes #461, fixes #480, fixes #481

Improves testing coverage, prioritizing runtime, transfer strategies, and work models. Also reorganizes the tests directory structure to mirror the src directory.

This PR also deprecates the increasing_connectivity ordering from the recursive transfer strategy.

I also added the option to output the subclustering test case to JSON--currently the write-out commands are commented out in test_lbs_clustering_transfer_strategy.py.

cwschilly commented 9 months ago

Current status: 52% coverage

Module statements missing excluded coverage
src/lbaf/Applications/LBAF_app.py 288 223 0 23%
src/lbaf/Applications/MoveCountsViewer.py 211 193 0 9%
src/lbaf/Applications/init.py 0 0 0 100%
src/lbaf/Execution/init.py 0 0 0 100%
src/lbaf/Execution/lbsAlgorithmBase.py 139 55 0 60%
src/lbaf/Execution/lbsBruteForceAlgorithm.py 38 28 0 26%
src/lbaf/Execution/lbsCentralizedPrefixOptimizerAlgorithm.py 160 54 0 66%
src/lbaf/Execution/lbsClusteringTransferStrategy.py 99 7 0 93%
src/lbaf/Execution/lbsCriterionBase.py 37 8 0 78%
src/lbaf/Execution/lbsInformAndTransferAlgorithm.py 111 13 0 88%
src/lbaf/Execution/lbsPhaseStepperAlgorithm.py 18 11 0 39%
src/lbaf/Execution/lbsRecursiveTransferStrategy.py 107 24 0 78%
src/lbaf/Execution/lbsRuntime.py 41 2 0 95%
src/lbaf/Execution/lbsStrictLocalizingCriterion.py 24 15 0 38%
src/lbaf/Execution/lbsTemperedCriterion.py 16 0 0 100%
src/lbaf/Execution/lbsTransferStrategyBase.py 78 5 0 94%
src/lbaf/IO/init.py 0 0 0 100%
src/lbaf/IO/lbsConfigurationUpgrader.py 130 130 0 0%
src/lbaf/IO/lbsConfigurationValidator.py 60 8 0 87%
src/lbaf/IO/lbsGridStreamer.py 65 60 0 8%
src/lbaf/IO/lbsStatistics.py 220 85 0 61%
src/lbaf/IO/lbsVTDataReader.py 154 36 0 77%
src/lbaf/IO/lbsVTDataWriter.py 59 47 0 20%
src/lbaf/IO/lbsVisualizer.py 492 469 0 5%
src/lbaf/Model/init.py 0 0 0 100%
src/lbaf/Model/lbsAffineCombinationWorkModel.py 28 1 0 96%
src/lbaf/Model/lbsBlock.py 32 7 0 78%
src/lbaf/Model/lbsLoadOnlyWorkModel.py 10 0 0 100%
src/lbaf/Model/lbsMessage.py 10 0 0 100%
src/lbaf/Model/lbsObject.py 79 12 0 85%
src/lbaf/Model/lbsObjectCommunicator.py 29 0 0 100%
src/lbaf/Model/lbsPhase.py 228 34 0 85%
src/lbaf/Model/lbsRank.py 122 15 0 88%
src/lbaf/Model/lbsWorkModelBase.py 19 0 0 100%
src/lbaf/Utils/init.py 0 0 0 100%
src/lbaf/Utils/lbsArgumentParser.py 98 86 0 12%
src/lbaf/Utils/lbsColors.py 30 4 0 87%
src/lbaf/Utils/lbsCsv2JsonDataConverter.py 101 76 0 25%
src/lbaf/Utils/lbsDataStatFilesUpdater.py 93 72 0 23%
src/lbaf/Utils/lbsJSONDataFilesValidatorLoader.py 41 10 0 76%
src/lbaf/Utils/lbsLogging.py 61 9 0 85%
src/lbaf/Utils/lbsPath.py 10 3 0 70%
src/lbaf/Utils/lbsVTDataExtractor.py 169 77 0 54%
src/lbaf/Utils/lbsWeb.py 31 11 0 65%
src/lbaf/init.py 33 12 0 64%
src/lbaf/main.py 7 7 0 0%
src/lbaf/imported/JSON_data_files_validator.py 125 11 0 91%
src/lbaf/imported/init.py 0 0 0 100%
Total 3903 1883 0 52%
ppebay commented 8 months ago

@cwschilly any update regarding current coverage after latest commits? Thanks

cwschilly commented 8 months ago

@cwschilly any update regarding current coverage after latest commits? Thanks

@ppebay I have updated the table. lbsClusteringTransferStrategy.py is now at 93% (up from 13% on develop) and total coverage is at 48% (up from 35%).

cwschilly commented 8 months ago

This test is failing, and I'm not sure what the problem is:

def test_recursive_transfer_strategy_orderings(self):

      # Define all order strategies
      order_strategy_list = ["arbitrary", "element_id", "decreasing_loads", "increasing_loads",
                             "increasing_connectivity", "fewest_migrations", "small_objects"]

      # Initialize empty parameter dict
      param_dict = {}

      # Set up received and sent objects
      rec_object_min = {Object(i=7, load=0.5): 5.0}
      rec_object_max = {Object(i=8, load=1.0): 10.0}
      sent_object_min = {Object(i=9, load=0.5): 6.0}
      sent_object_max = {Object(i=10, load=1.0): 12.0}

      # Create objects
      obj_04 = Object(i=4, load=5.0, comm=ObjectCommunicator(i=4,logger=self.logger,r=rec_object_max, s=sent_object_max))
      obj_05 = Object(i=5, load=3.0, comm=ObjectCommunicator(i=5,logger=self.logger,r=rec_object_min, s=sent_object_min))
      obj_06 = Object(i=6, comm=None)

      # Define objects set
      objects = [
        obj_04,
        obj_05,
        obj_06
      ]

      # Set expected orders
      expected_order_dict = {
          "arbitrary": objects,
          "element_id": objects,
          "decreasing_loads": objects,
          "increasing_loads": objects[::-1],
          "increasing_connectivity": objects[::-1],
          "fewest_migrations": objects,
          "small_objects": objects
      }

      # Test every order strategy
      for order_strategy in order_strategy_list:
          param_dict["order_strategy"] = order_strategy
          recursive_strat = RecursiveTransferStrategy(criterion=self.criterion, parameters=param_dict, logger=self.logger)
          self.assertEqual(f"{order_strategy}: {getattr(recursive_strat, order_strategy)(objects, 0)}",
                           f"{order_strategy}: {expected_order_dict[order_strategy]}")

This is the error that it produces:

AssertionError: 'incr[34 chars], load: 0.0, Object id: 4, load: 5.0, Object id: 5, load: 3.0]' != 'incr[34 chars], load: 0.0, Object id: 5, load: 3.0, Object id: 4, load: 5.0]'
- increasing_connectivity: [Object id: 6, load: 0.0, Object id: 4, load: 5.0, Object id: 5, load: 3.0]
?                                                               ^        ^               ^        ^
+ increasing_connectivity: [Object id: 6, load: 0.0, Object id: 5, load: 3.0, Object id: 4, load: 5.0]

In other words, ordering the objects by increasing_connectivity does not yield what I would expect. (I expected 6, 5, 4 but it is actually 6, 4, 5)

@ppebay Am I misunderstanding what the expected output should be?

For reference, here is the full increasing_connectivity function from lbsRecursiveTransferStrategy:

@staticmethod
def increasing_connectivity(objects: set, src_id):
    """Order objects by increasing local communication volume."""
    # Initialize list with all objects without a communicator
    no_comm = [
        o for o in objects
        if not isinstance(o.get_communicator(), ObjectCommunicator)]

    # Order objects with a communicator
    with_comm = {}
    for o in objects:
        # Skip objects without a communicator
        comm = o.get_communicator()
        if not isinstance(o.get_communicator(), ObjectCommunicator):
            continue

        # Update dict of objects with maximum local communication
        with_comm[o] = max(
            sum([v for k, v in comm.get_received().items()
                 if k.get_rank_id() == src_id]),
            sum([v for k, v in comm.get_sent().items()
                 if k.get_rank_id() == src_id]))

    # Return list of objects order by increased local connectivity
    return no_comm + sorted(with_comm, key=with_comm.get)
ppebay commented 8 months ago

@cwschilly this is an interesting finding, because I do not remember what increased_connectivity was intended for.

I also do not understand how we could use it in this context because the load is not related to connectivity, therefore it's not possible to tell just from load alone could be used to determine the order...

ppebay commented 8 months ago

@cwschilly looking at this:

# Update dict of objects with maximum local communication

makes me think that we would have to reverse-engineer this, for I do not remember what it was meant for. I would even venture to say that in fell in disuse. Please try to figure if it's used anywhere -- otherwise let's just deprecate it. Thanks