CharJon / GeCO

Generators for Combinatorial Optimization
MIT License
14 stars 4 forks source link

Shuffle instance function #81

Closed mmghannam closed 3 years ago

CharJon commented 3 years ago

I think we should add a testcase, that checks if writeProblem produces two different files.

CharJon commented 3 years ago

Also, I see a downside of this approach vs the model being shuffled before hand. If the shuffle is done by the parameters, calling "resetParams" on the model kind of removes the shuffle, which might be unexpected.

codecov-io commented 3 years ago

Codecov Report

Merging #81 (5750da1) into main (153bc70) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main       #81   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           40        41    +1     
  Lines          694       701    +7     
=========================================
+ Hits           694       701    +7     
Impacted Files Coverage Δ
geco/mips/utilities/generic.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 153bc70...bfb8f80. Read the comment docs.

CharJon commented 3 years ago

I'm not sure, if we should keep the test that uses optimize to check if shuffling works.

If we want to keep a test like this, we should have a persistent test file in e.g. data/test-data

mmghannam commented 3 years ago

I'm not sure, if we should keep the test that uses optimize to check if shuffling works.

  • It heavily depends on the yang instances and therefore has hidden side effects: If the yang implementation changes and the problem becomes easy (1 node) the test fails, even if shuffling might work.
  • We are testing if the solution process changes and not if shuffling works. So from a testing theory perspective we are testing the wrong thing.
  • It is slow (for a test).

If we want to keep a test like this, we should have a persistent test file in e.g. data/test-data

I agree, I will remove it.