-
Notifications
You must be signed in to change notification settings - Fork 32
Restructure ClassificationGAM 2
#272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| ## A 2-element numeric vector specifying the prior probabilities for each | ||
| ## class. The order of the elements in @qcode{Prior} corresponds to the | ||
| ## order of the classes in @qcode{ClassNames}. This property is read-only. | ||
| ## A numeric vector specifying the prior probabilities for each class. The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to change the property docstring. It is always a 2-element numeric vector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I investigated matlab's working with Prior, so modified for MATLAB compatibility.
| switch (s.subs) | ||
| case 'Cost' | ||
| this.Cost = setCost (this, val); | ||
| this.Cost = val; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep my previous change utilizing a priveate method so it can also be called from subsasgn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried using that, but when I tested, it was causing errors, I tried 2-3 ways but it didnt work, so came up with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it doesn't work because my previous code was wrong. Instead of
this.Cost = setCost (this, val);
it should have been
this = setCost (this, val);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So should I keep the current changes, since it is working well right?
| Formula = []; | ||
| Interactions = []; | ||
| ClassNames = []; | ||
| Prior = "empirical"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prior must be a 2-element numeric vector. Don't initialize it like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same mentioned reason of MATLAB compatibility.
| RSS = sum (res .^ 2); | ||
| endfunction | ||
|
|
||
| function this = setCost (this, Cost, gnY = []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep this private method. See my comment above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same above mentioned reason for removing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my comment above
| %! assert (a.DoF, [7, 7, 7]) | ||
| %! assert (a.BaseModel.Intercept, 0.4055, 1e-1) | ||
|
|
||
| ## Test Prior calculation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to document this name-value argument in the constructor's help docstring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## @item @qcode{'Prior'} @tab @tab A numeric vector specifying the prior
## probabilities for each class. The order of the elements in @qcode{Prior}
## corresponds to the order of the classes in @qcode{ClassNames}.
## Alternatively, you can specify @qcode{"empirical"} to use the empirical
## class probabilities or @qcode{"uniform"} to assume equal class
## probabilities.
Is this what you are talking about? If yes, then it is already added in the file.
No description provided.