Replies: 9 comments
-
|
@jessdtate @zaracay @moritzdannhauer Thoughts? |
Beta Was this translation helpful? Give feedback.
-
|
Yes, a more careful check is needed and I think there are ways (in Eigen) to check for these cases efficiently. We also should put some checking routines in place for the second input of the module and test its functionality again. |
Beta Was this translation helpful? Give feedback.
-
|
I think a check would be useful. It shouldn’t be necessary, but I could foresee scenarios in which people would try to put in inf or nan values and so, we should probably have something built in to check. The second input is supposed to accept a look up table of conductivity values, so checking that for inf and nan makes sense as well. Brett Burton PhD Candidate in Bioengineering | University of Utah
|
Beta Was this translation helpful? Give feedback.
-
|
I have lately come to favor only passing errors when it will break the code. I think that it should be in the modules that cannot handle them, ie, the solver and addknowns (which already has a check apparently), and then add to the error message more info. A warning, rather than an error, in BuildFEMatrix would be better as long as it isn't too expensive. Though our solver doesn't explicitly handle those values, there may be some applications where it is useful, assuming that it does it correctly. If it makes no mathematical sense, or we aren't giving the correct answer then we shouldn't allow it. |
Beta Was this translation helpful? Give feedback.
-
|
I see logic in what Jess is saying. I don’t know if it makes sense mathematically to allow inf or nan values. It doesn’t make sense in any of the cases that I work with, but I’ve never thrown a perfect conductor into a model (inf conductivity), and I’ve never tried to solve for unknown conductivites. We should talk to Steve Maas or Dave Rawlins about this to see if such conditions should ever be allowed in FE matrix construction. If it shouldn’t be allowed, we have BuildFE throw an error. If it is possible to include them, we have it throw a warning. I would say it is a relatively low priority, though Brett Burton PhD Candidate in Bioengineering | University of Utah
|
Beta Was this translation helpful? Give feedback.
-
|
I am not aware of any case where inf or nan values would make sense mathematically. If one wants to model an infinite conductivity one should use addlinkednodestolinearsystem, trying to do this using inf values for your conductivity messes up the complete matrix condition, furthermore I have no idea what this would do to the solver. nan values for the conductivity don't make sense anyway. |
Beta Was this translation helpful? Give feedback.
-
|
So we return to the question, should we put a check into the BuildFEMatrix module for inf/nan values? My vote is yes, if it doesn’t require a lot of work. New users won’t necessarily know about the “AddLinkedNodes…" module, and, therefore might try to input inf values. It is, however, a low priority feature. Brett Burton PhD Candidate in Bioengineering | University of Utah
|
Beta Was this translation helpful? Give feedback.
-
|
@jessdtate Keep open or something to do? |
Beta Was this translation helpful? Give feedback.
-
|
I think we should do it if we can, but I think it is still low priority. |
Beta Was this translation helpful? Give feedback.
-
When having inf or nan values in the conductivity vector, BuildFEMatrix doesn't throw an error, instead AddKnownsToLinearSystem throws the error "NaN exist in the b vector". Since inf values can relatively easy occur when being careless during rescaling of conductivities (e.g., when computing them from DTI data), this might be a very helpful check. The current error obviously is not very helpful to solve the problem. A drawback would of course be that this additional check costs performance, so this would be a decision between performance and usability.
Versions
v5.0-beta.D opensuse leap
Beta Was this translation helpful? Give feedback.
All reactions