New models for softmax#355
Conversation
347a9e3 to
8042b51
Compare
pobonomo
added
safe to test
|
Hi Pierre: just wanted to check if you have an estimate for when you think this will be merged. I'd love to use it! No pressure at all. Or as you indicate it's safe to test, so I could locally clone this PR and play around with it? Thanks. |
df973ca to
16c95a9
Compare
pobonomo
added
Models
and removed
safe to test
aa2ef69 to
0c981b3
Compare
pobonomo
added
the
safe to test
Hi, I have no estimate currently. The models are working I think but it's a big change and there is a lot of cleanup. Note that what is implemented so far is using softmax for scikit-learn only: multinomial logistic regression and neural networks. I didn't implement counter-parts for Keras and Pytorch yet (the code should be very simple but there would be a lot of testing). I think that it is usable though so if you want to play with it you could try (and more testing cannot hurt). Best, |
95f7a20 to
3c35471
Compare
|
Thanks, will do! |
363b38d to
ad4c84c
Compare
pobonomo
added
safe to test
dbab218 to
327d178
Compare
2259241 to
f75c9e6
Compare
1830edf to
86c8f73
Compare
|
|
b839c8a to
5603e09
Compare
5603e09 to
d7e2a17
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 43 changed files in this pull request and generated 17 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -74,7 +79,12 @@ class PipelineConstr(SKgetter, AbstractPredictorConstr): | |||
| def __init__(self, gp_model, pipeline, input_vars, output_vars=None, **kwargs): | |||
| self._steps = [] | |||
| self._default_name = "pipe" | |||
| SKgetter.__init__(self, pipeline, input_vars, **kwargs) | |||
| if hasattr(pipeline, "predict_proba"): | |||
| SKClassifier.__init__(self, pipeline, input_vars, **kwargs) | |||
| self._isclassifier = True | |||
| else: | |||
| SKRegressor.__init__(self, pipeline, input_vars, **kwargs) | |||
| self._isclassifier = False | |||
There was a problem hiding this comment.
The diamond inheritance pattern (SKRegressor, SKClassifier both inherit from SKgetter) combined with conditional initialization could lead to unexpected behavior. While the comment acknowledges this, the Method Resolution Order (MRO) means SKRegressor.init will be called first regardless of the conditional. Consider using composition instead of multiple inheritance, or ensure the parent classes properly use super() to handle the diamond pattern correctly.
| else: | ||
| tol = 1e-5 | ||
| vio = gpm.MaxVio | ||
| tol = 5e-3 |
There was a problem hiding this comment.
The tolerance is set based on the nonconvex parameter (line 80-83), but then immediately overwritten to 5e-3 on line 84 regardless of the nonconvex value. This makes the conditional logic on lines 80-83 dead code. Either remove the conditional or remove line 84 if it was added for debugging.
| tol = 5e-3 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 43 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5c8135d to
b8b538b
Compare
9da9c74 to
bc62309
Compare
Introduce softmax layer modeling for neural networks and extend LogisticRegression to support multinomial classification (one-vs-rest and multinomial/softmax modes). Key changes: - Add softmax.py module for softmax layer formulation - Refactor LogisticRegression to handle multinomial classification - Extend activation functions with softmax support - Add adversarial example notebooks - Update predictor getter to support classification mip-models
Extend neural network modeling to support MLPClassifier from scikit-learn, enabling multi-class classification with neural networks in optimization models. This builds on the softmax infrastructure to handle the classifier's output layer appropriately. Changes: - Add MLPClassifierConstr class in mlpregressor.py module - Handle both regression and classification MLP models - Add iris dataset test cases for MLPClassifier - Update MNIST examples to use MLPClassifier instead of MLPRegressor
If predict_function is identity we don't apply any softmax
It is not used anymore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Do new models for softmax