-
Notifications
You must be signed in to change notification settings - Fork 62
Delete lock interface implementation from UmfpackLU #617
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #617 +/- ##
==========================================
+ Coverage 83.95% 84.01% +0.06%
==========================================
Files 12 12
Lines 9265 9256 -9
==========================================
- Hits 7778 7776 -2
+ Misses 1487 1480 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I'm not critical of the patch, but why? IMHO, it's fair to warn people that they are waiting for locks to do sparse operations. |
The motivation wasn't really to remove the warning but rather that it is kind of weird that a struct Regarding the warning, since it isn't really actionable by the user, and safe to ignore, it seems kind of pointless? |
I think it would make sense to document it rather than give a warning. |
Makes sense
It is actionable, you can pass a different workspace.
Nobody expects |
Perhaps it would be better with a documentation section for multithreaded use then? The warning doesn't tell the user what to do or how to do it so that is what I mean with unactionable. Since JuliaLang/julia#52883 you can also see lock contention in |
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 agree with both things here. It is weird to implement these lock methods for UmfpackLU
, if anything a user should probably use Lockable
and for the warning message it isn't something that is typically done in Julia libraries.
No description provided.