Skip to content

Hashir changes#1

Merged
zkhotanlou merged 2 commits intomainfrom
hashir-changes
Feb 2, 2026
Merged

Hashir changes#1
zkhotanlou merged 2 commits intomainfrom
hashir-changes

Conversation

@zkhotanlou
Copy link
Owner

No description provided.

The results are not the same as the paper, but seem to be consistent
to some degree. I believe the actual method is implemented correctly,
but there are differences in the way the datasets are processed and
the models are trained, which can lead to different results.
@zkhotanlou
Copy link
Owner Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines +175 to +182
optimizer.zero_grad()
# x_new_enc = reconstruct_encoding_constraints(
# x_new, cat_feature_indices, binary_cat_features
# )
# use x_new_enc for prediction results to ensure constraints
# f_x_new = softmax(torch_model(x_new))[:, 1]
f_x_new_binary = torch_model(x_new).squeeze(axis=0)

Choose a reason for hiding this comment

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

P0 Badge Replace unsupported axis argument to squeeze

Inside the optimization loop the code calls torch_model(x_new).squeeze(axis=0) before computing the loss. PyTorch tensors only accept the keyword dim (or a positional argument) for squeeze; passing axis raises a TypeError, so Probe.get_counterfactuals will fail as soon as it is executed. Use squeeze(0) or squeeze(dim=0) instead so the loop can run.

Useful? React with 👍 / 👎.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@codex just to clarify, even with Python 3.7 and PyTorch 1.7.0, this syntax would still be invalid?

Choose a reason for hiding this comment

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

Copy link
Owner Author

Choose a reason for hiding this comment

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

@codex even with Python 3.7 and PyTorch 1.7.0, this syntax would still be invalid?

Choose a reason for hiding this comment

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

Summary

Testing

  • ⚠️ No automated tests were run (not requested).

View task →

@zkhotanlou zkhotanlou merged commit 0281cc1 into main Feb 2, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants