-
Notifications
You must be signed in to change notification settings - Fork 76
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
Resolve softmax issues with pytorch_model.py #283
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #283 +/- ##
==========================================
+ Coverage 93.29% 93.35% +0.06%
==========================================
Files 62 62
Lines 3430 3448 +18
==========================================
+ Hits 3200 3219 +19
+ Misses 230 229 -1
|
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.
A few things! Thanks @dilyabareeva !
@@ -5,15 +5,15 @@ | |||
# Quantus is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. |
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.
Can you double-check the tf implementation so that the function names as well as the logic are replicated there?
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.
Both use the same ModelInterface - so the public methods are the same. The logic in tf_model might need to be changed similarly: I'm not 100% sure that model.layers[-1] reliably returns the actual output layer in tensorflow. But this PR is about PyTorch, so maybe this is worth opening another issue to further investigate.
@dilyabareeva You are modifying the structure of the model provided by the user, it would be great to make it visible to the user what exactly is being done under the hood, and what assumptions were made.
This must not be a warning, since this is expected behaviour. I'd suggest using |
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.
OK.
Description
Implemented changes