Libraries Upgrade et al.#150
Conversation
| plt.plot(fpr, tpr, color="darkorange", lw=2, label=f"ROC curve (AUC = {roc_auc:.4f})") | ||
| plt.plot([0, 1], [0, 1], color="navy", lw=2, linestyle="--") | ||
| plt.xlim([0.0, 1.0]) | ||
| plt.ylim([0.0, 1.05]) |
There was a problem hiding this comment.
Mypy complained about this.
| } | ||
| targets: dict[str, np.ndarray] = { | ||
| DataSplit.TRAIN.value: data[[table_metadata.target_column_name]].values.astype(np.float32) | ||
| } |
There was a problem hiding this comment.
Mypy complained about the typing.
| if ( | ||
| is_string_dtype(dataframe[column_name]) | ||
| or is_object_dtype(dataframe[column_name]) | ||
| or (is_column_type_numerical(dataframe, column_name) and dataframe[column_name].nunique() <= threshold) |
There was a problem hiding this comment.
You'll see this throughout, but Pandas typing for strings has changed quite a bit. Now they have a separate string type you can use, along with the object type. So this catches that.
| self.validate_dataframe_dtypes(real_data) | ||
| self.validate_dataframe_dtypes(synthetic_data) | ||
| if holdout_data is not None: | ||
| self.validate_dataframe_dtypes(holdout_data) |
There was a problem hiding this comment.
Added these validations to make sure that string types are not sneaking into our metrics computations. These were added wherever strings would cause issues.
| do_preprocessing=False, | ||
| verbose=False, | ||
| nn_dist=self.norm.value, | ||
| plot_figures=False, |
There was a problem hiding this comment.
In the new SynthEval, they generated plots by default, which was very annoying. This shuts them off.
| ) | ||
|
|
||
| return self.syntheval_metric.evaluate(self.confidence_level.value) | ||
| return self.syntheval_metric.evaluate(ci="sem", confidence=self.confidence_level.value) |
There was a problem hiding this comment.
New upgrade changed this method signature a bit. So we're forcing consistency here.
There was a problem hiding this comment.
For some reason, SynthEval changed the way they report the metric output here from nested dictionaries to pandas dataframes and changed a bunch of the key names. Very annoying, but these changes just address that new reporting structure and keep our output structures the same.
| end = min((i + 1) * batch_size, a_array.shape[0]) | ||
| distance, search_indices = index.search(a_array[start:end], k=1) | ||
| index.remove_ids(search_indices.flatten()) | ||
| index.remove_ids(search_indices.flatten()) # type: ignore |
There was a problem hiding this comment.
I could not, for the life of me, figure out the right type here. The faiss library is a mess.
| shutil.rmtree(results_dir, ignore_errors=True) | ||
|
|
||
|
|
||
| @pytest.mark.integration_test() |
There was a problem hiding this comment.
This wasn't marked integration, even though it is 🙂
|
|
||
|
|
||
| def test_train_attack_classifier_mismatched_data(attack_data): | ||
| @pytest.mark.parametrize("classifier_type", [ClassifierType.XGBOOST, ClassifierType.CATBOOST]) |
There was a problem hiding this comment.
Mocking these to make it faster.
| mapped_holdout_data = HOLDOUT_DATA.replace({"cat": 0, "horse": 1, "dog": 2}) | ||
| mapped_real_data = REAL_DATA.replace({"cat": 0, "horse": 1, "dog": 2}).astype(int) | ||
| mapped_synthetic_data = SYNTHETIC_DATA.replace({"cat": 0, "horse": 1, "dog": 2}).astype(int) | ||
| mapped_holdout_data = HOLDOUT_DATA.replace({"cat": 0, "horse": 1, "dog": 2}).astype(int) |
There was a problem hiding this comment.
This replace, for some reason, doesn't reclassify the column as an integer, even though it maps all categorical values to integers and seems like it should. So forcing it.
| # Now add some NaNs to the train and validation splits | ||
| dataset.numerical_features["train"][0, 1] = np.NaN | ||
| dataset.numerical_features["val"][1, 1] = np.NaN | ||
| dataset.numerical_features["train"][0, 1] = np.nan |
There was a problem hiding this comment.
np.NaN is no longer supported.
| tests/integration/assets/tabsyn/results | ||
|
|
||
| # Emitted SynthEval analysis config file during metric creation. Unfortunately cannot be turned off... | ||
| SE_analysis_config.json |
There was a problem hiding this comment.
New Syntheval emits this json file and it cannot be disabled after looking at their code...
| "Label column should not be included in the set of numerical columns provided" | ||
| "Label column should not be included in the set of categorical columns provided" | ||
| ) | ||
| categorical_columns = categorical_columns + [label_column] |
There was a problem hiding this comment.
Adding the label column here to make sure it gets preprocessed with the cat columns, but maintain the entry points of having label column separate to make it backwards consistent and also match the corresponding regression metric.
lotif
left a comment
lotif
left a comment
There was a problem hiding this comment.
Generally good to go but a few questions/suggestions.
A few things in this PR:
Adding an example for TabSyn full pipeline (train, synthesize and evaluate)
Adding an example for Ensemble Attack using TabSyn
Adding EnsembleAttackTabSynModelRunner to src/midst_toolkit/attacks/ensemble/models.py so the attack knows how to run the model
Renaming test_attack_model.py files to stop pytest thinking they are test files
A few small refactorings to simplify things
FYI a lot of the code in the examples was copied from the CTGAN example, and the EnsembleAttackTabSynModelRunner was adapted from the existing TabSyn automated test.
Also merging #150 and updating pillow v12.2.0 -> v12.3.0
2 participants
PR Type
Fix
Short Description
Clickup Ticket(s): N/A
Upgrading the libraries to address vulnerabilities. This required some code changes with the new libraries functionality and new APIs. None of the fixes were major, but did require some digging and test fixes.
Tests Added
Test fixes associated with the required changes were made. Some small changes to the deterministic tests were also required with the move to single threading for the tests.