-
-
Notifications
You must be signed in to change notification settings - Fork 316
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
i.gensigset: fix possible pole and divide by zero errors in regroup #4500
Open
ymdatta
wants to merge
3
commits into
OSGeo:main
Choose a base branch
from
ymdatta:cppcheck_imagery_log2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+11
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ymdatta
commented
Oct 11, 2024
imagery/i.gensigset/subcluster.c
Outdated
if (subsum > 0) | ||
likelihood += log(subsum) + maxlike; | ||
|
||
if (subsum) { |
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.
Didn't combine both of these in the same conditional as subsum
being negative is not valid for log
, but valid while calculating Data
.
Using logarithm function call with zero argument will lead to a pole error, which occurs if the mathematical function has an exact infinite result. Check if the argument value is zero before passing that to the log function to avoid such errors. I also added check for negative numbers just to make sure the argument is in the right domain for the log function as well. There was also a possible divide by zero scenario when we were dividing the class data by subsum, which can be zero. Added a conditional check which avoids going to that stage. Signed-off-by: Mohan Yelugoti <[email protected]>
ymdatta
force-pushed
the
cppcheck_imagery_log2
branch
from
October 23, 2024 21:41
3f08d03
to
2b1b9e6
Compare
ymdatta
added a commit
to ymdatta/grass
that referenced
this pull request
Nov 27, 2024
Documented each supression issue with comments to distinguish between false positives and true positives awaiting resolution. For the false positives supressions, appropriate information is provided on why those were considered as false positive. True positives will be removed from the suppression file once their corresponding fixes(OSGeo#4702, OSGeo#4638, OSGeo#4500, OSGeo#4499) are merged. Run: `cppcheck --suppressions-list=.cppcheck-supressions <path>` Signed-off-by: Mohan Yelugoti <[email protected]>
ymdatta
added a commit
to ymdatta/grass
that referenced
this pull request
Nov 27, 2024
Documented each suppression issue with comments to distinguish between false positives and true positives awaiting resolution. For the false positives suppressions, appropriate information is provided on why those were considered as false positive. True positives will be removed from the suppression file once their corresponding fixes(OSGeo#4702, OSGeo#4638, OSGeo#4500, OSGeo#4499) are merged. Run: `cppcheck --suppressions-list=.cppcheck-suppressions <path>` Signed-off-by: Mohan Yelugoti <[email protected]>
Maybe a fresh, updated CI run might be wanted before a merge, last run is getting old |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using logarithm function call with zero argument will lead to a pole error, which occurs if the mathematical function has an exact infinite result.
Check if the argument value is zero before passing that to the log function to avoid such errors. I also added check for negative numbers just to make sure the argument is in the right domain for the log function as well.
There was also a possible divide by zero scenario when we were dividing the class data by subsum, which can be zero. Added a conditional check which avoids going to that stage.