DARMA-tasking / LB-analysis-framework

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

#542: Fix as many pylint issues as possible before release #546

Closed cwschilly closed 2 months ago

cwschilly commented 2 months ago

Fixes #542

These are all "easy" fixes; warnings that would require more refactoring of the code (e.g. too-many-*) have not been resolved.

cwschilly commented 2 months ago

Thanks @tlamonthezie , I fixed the warnings in lbsPrescribedPermutationAlgorithm.py.

I left the lbsVisualizer as it is because (like you said) it'll be removed very soon.

As for the estimate warning, it looks like CriterionBase defines this as a method, but it's not clear to me whether it's really used.

def estimate(self, r_src: Rank, o_src: list, *args) -> float:
        """Estimate is compute because all information is local for this criterion."""
        return self.compute(r_src, o_src, *args)

It seems then that we could just remove the estimate function entirely. I mentioned this to @ppebay -- I think this will require further investigation outside of this PR.

ppebay commented 2 months ago

Thanks @tlamonthezie , I fixed the warnings in lbsPrescribedPermutationAlgorithm.py.

I left the lbsVisualizer as it is because (like you said) it'll be removed very soon.

As for the estimate warning, it looks like CriterionBase defines this as a method, but it's not clear to me whether it's really used.

  • TemperedCriterion doesn't implement it at all (which causes the warning)
  • StrictLocalizingCriterion implements it by simply forwarding it to the compute function:
def estimate(self, r_src: Rank, o_src: list, *args) -> float:
        """Estimate is compute because all information is local for this criterion."""
        return self.compute(r_src, o_src, *args)

It seems then that we could just remove the estimate function entirely. I mentioned this to @ppebay -- I think this will require further investigation outside of this PR.

Yes, please create follow-on issue @cwschilly and let's leave it in the code for now. Thanks