Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,6 @@ cython_debug/

# PyPI configuration file
.pypirc

# Mac OS
.DS_Store
39 changes: 39 additions & 0 deletions notebooks/run_xp.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import sys

print(sys.executable)
print("ole")
print(sys.version)
import matplotlib.pyplot as plt
import gurobipy as hp
sys.path.append("../")


import pickle
import numpy as np

from python.data_generation import SyntheticDataGenerator
from python.distances import TwoUTASpaceDiameter

alldist = {}
for ndata in [10, 20, 100, 1_000]:
print("Nb of data:", ndata)
generator = SyntheticDataGenerator(
n_dms=2,
n_criteria=4,
method_params={"n_pieces": 5},
)

X, Y, info = generator.generate_preferences(num_pairs=ndata, return_clusters=True)
dist = TwoUTASpaceDiameter(n_pieces=5)
dist.fit(X, Y)

alldist[ndata] = {
"s1": [[dist.marginal_coeffs["s1", i, k].x for k in range(6)] for i in range(4)],
"s2": [[dist.marginal_coeffs["s2", i, k].x for k in range(6)] for i in range(4)],
"d1": [[dist.marginal_coeffs["d1", i, k].x for k in range(6)] for i in range(4)],
"d2": [[dist.marginal_coeffs["d2", i, k].x for k in range(6)] for i in range(4)],
"objval": dist.solver.objVal
}

with open('filename_bis.pickle', 'wb') as handle:
pickle.dump(alldist, handle, protocol=pickle.HIGHEST_PROTOCOL)
253 changes: 253 additions & 0 deletions notebooks/synthetic_experiments.ipynb

Large diffs are not rendered by default.

171 changes: 171 additions & 0 deletions python/data_generation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import numpy as np

from .decision_maker import DecisionMaker

class SyntheticDataGenerator:
def __init__(
self,
n_dms,
n_criteria,
mix_decisions=False,
method_params={},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using a mutable default argument like {} can lead to unexpected behavior where modifications to the dictionary in one instance affect all others. It's safer to use None as the default and then initialize self.method_params to method_params if method_params is not None else {} inside __init__.

Suggested change
method_params={},
method_params=None,

noise=0.0,
gap=0.0,
decimals=6,
):
self.n_dms = n_dms
self.n_criteria = n_criteria
self.mix_decisions = mix_decisions
self.method_params = method_params

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To avoid issues with mutable default arguments, initialize self.method_params like this, which will create a new dictionary for each instance when no method_params are provided.

Suggested change
self.method_params = method_params
self.method_params = method_params if method_params is not None else {}

self.noise = noise # % of noise = % of pairs that will be reversed
self.gap = gap
self.decimals = decimals

self.instantiate()

def instantiate(self):
self.dms = [DecisionMaker(n_criteria=self.n_criteria, n_pieces=self.method_params.get("n_pieces", 5)) for _ in range(self.n_dms)]

self._marginal_utilities = lambda x: np.array([[dm.get_marginal_utility(criterion_index=i, criterion_value=x[i]) for i in range(len(x))] for dm in self.dms])
self._utility = lambda x: np.array([dm.get_total_utility(criteria_vector=x) for dm in self.dms])

def utility(self, X):
if len(X.shape) == 1:
return self._utility(X)
elif len(X.shape) == 2:
return np.array([self._utility(x) for x in X])
else:
raise ValueError("Unsupported shape of X", X.shape)

def marginal_utilities(self, X):
if len(X.shape) == 1:
return self._marginal_utility(X)
elif len(X.shape) == 2:
return np.array([self._marginal_utility(x) for x in X])
else:
raise ValueError("Unsupported shape of X", X.shape)

def generate_preferences(
self, num_pairs, return_utilities=False, return_clusters=False, verbose=0
):
X, Y = [], []

utilities = [[], []]
clusters = []
# Useless now that we have clusters
populations = [0] * self.n_dms
if not isinstance(num_pairs, list):
num_pairs = [np.ceil(num_pairs / self.n_dms)] * self.n_dms
Comment on lines +55 to +58

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are a couple of issues here:

  1. The comment on line 53, "Useless now that we have clusters", is misleading as populations is used later in the method. It should be updated or removed.
  2. On line 56, num_pairs is calculated using np.ceil, which results in a list of floats. It's safer to convert these to integers using .astype(int) as done in generate_indifference_data on line 129 to avoid potential issues.
Suggested change
# Useless now that we have clusters
populations = [0] * self.n_dms
if not isinstance(num_pairs, list):
num_pairs = [np.ceil(num_pairs / self.n_dms)] * self.n_dms
# The `populations` list tracks the number of pairs generated for each DM cluster.
populations = [0] * self.n_dms
if not isinstance(num_pairs, list):
num_pairs = np.array([np.ceil(num_pairs / self.n_dms)] * self.n_dms).astype(int)

while len(X) < sum(num_pairs):
if verbose > 0:
print(f"{len(X)} events have been created as of now", end="\r")

non_dominance = False
while not non_dominance:
x = np.around(
np.random.uniform(0, 1, self.n_criteria), decimals=self.decimals
)
y = np.around(
np.random.uniform(0, 1, self.n_criteria), decimals=self.decimals
)
non_dominance = (np.sum(x-y > 0) != len(x)) & (np.sum(x-y > 0) != 0)

ux = np.around(self.utility(x), decimals=self.decimals)
uy = np.around(self.utility(y), decimals=self.decimals)
if (ux - uy)[np.argmax(ux - uy)] > self.gap:
if np.sum(ux > uy) == 1 and not self.mix_decisions:
if populations[np.argmax(ux > uy)] < num_pairs[np.argmax(ux > uy)]:
if np.random.randint(1000) / 1000 >= self.noise:
X.append(x)
Y.append(y)
utilities[0].append(ux)
utilities[1].append(uy)
else:
X.append(y)
Y.append(x)
utilities[0].append(uy)
utilities[1].append(ux)

populations[np.argmax(ux - uy)] += 1
clusters.append(np.argmax(ux - uy))

elif np.sum(ux > uy) >= 1 and self.mix_decisions:
if populations[np.argmax(ux - uy)] < num_pairs[np.argmax(ux - uy)]:
if np.random.randint(1000) / 1000 >= self.noise:
X.append(x)
Y.append(y)
utilities[0].append(ux)
utilities[1].append(uy)
else:
X.append(y)
Y.append(x)
utilities[0].append(uy)
utilities[1].append(ux)

populations[np.argmax(ux - uy)] += 1
clusters.append(np.argmax(ux - uy))
Comment on lines +76 to +106

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The code inside the if and elif blocks is almost identical, leading to duplication. This can be refactored into a single block by combining the conditions. This would improve maintainability.

Additionally, np.random.randint(1000) / 1000 can be replaced with the more idiomatic np.random.random() for generating a random float.

if verbose > 0:
print("Clusters Populations", populations)
additional_info = {}
for i in range(self.n_dms):
additional_info[f"coefficients_{i}"] = self.dms[i].coefficients

if return_utilities:
additional_info["utilities_x"] = np.array(utilities)[0]
additional_info["utilities_y"] = np.array(utilities)[1]
if return_clusters:
additional_info["clusters"] = np.array(clusters)
return np.stack(X), np.stack(Y), additional_info


def generate_indifferences(
self, num_pairs, return_utilities=False, return_clusters=False, verbose=0
):
X, Y = [], []

utilities = [[], []]
clusters = []
# Useless now that we have clusters
populations = [0] * self.n_dms
if not isinstance(num_pairs, list):
num_pairs = np.array([np.ceil(num_pairs / self.n_dms)] * self.n_dms).astype(int)

for i in range(self.n_dms):
for _ in range(num_pairs[i]):
if verbose > 0:
print(f"{len(X)} events have been created as of now", end="\r")
x = np.around(
np.random.uniform(0, 1, self.n_criteria), decimals=self.decimals
)
ux = np.around(self.utility(x), decimals=self.decimals)[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The utility ux is always calculated for the first decision maker ([0]) instead of the current one in the loop (i). This is likely a bug. It should use [i] to be consistent with the logic for uy on line 155.

Suggested change
ux = np.around(self.utility(x), decimals=self.decimals)[0]
ux = np.around(self.utility(x), decimals=self.decimals)[i]

y = x.copy()

uyi_1 = None
while uyi_1 is None:
indexes = np.random.permutation(np.arange(len(x)))[:2]

uyi_0 = np.random.uniform(0, 1)
uyi_1 = self.dms[i].get_indifference_on_two_criteria(criterion_i=indexes[0], criterion_j=indexes[1],
query_i=x[indexes[0]], p_i=x[indexes[1]], query_j=uyi_0)

y[indexes[0]] = uyi_0
y[indexes[1]] = uyi_1

X.append(x)
Y.append(y)
utilities[0].append(ux)
utilities[1].append(np.around(self.utility(y), decimals=self.decimals)[i])
populations[i] += 1
clusters.append(i)
if verbose > 0:
print("Clusters Populations", populations)
additional_info = {}
for i in range(self.n_dms):
additional_info[f"coefficients_{i}"] = self.dms[i].coefficients

if return_utilities:
additional_info["utilities_x"] = np.array(utilities)[0]
additional_info["utilities_y"] = np.array(utilities)[1]
if return_clusters:
additional_info["clusters"] = np.array(clusters)
return np.stack(X), np.stack(Y), additional_info
Loading
Loading