SCIInstitute / SCIRun

SCIRun is a Problem Solving Environment, for modeling, simulation and visualization of scientific problems. This is version 5, the upgraded version of SCIRun v4.
http://scirun.org
Other
128 stars 72 forks source link

SetFieldData doesn't produce error when matrix/field doesn't match up #1152

Open dcwhite opened 9 years ago

dcwhite commented 9 years ago

It just outputs a null field instead.

dcwhite commented 9 years ago

When the matrix has too few matrix values (rows), an error is produced but it's the wrong message: Internal error (data not tensor). When there are too many rows, the data is all set to zero. Both cases should be tested and fixed.

dcwhite commented 9 years ago

Found out more about what's going on: algo is falling into a case where it's trying to match a single tensor in a matrix (i.e. 1x9 or 1x6) to all node values, but the field isn't set to accept tensors. These cases should be cleared up somehow.

SeyhmusGuler commented 8 years ago

It doesn't give an error when data is a sparse matrix. ReportFieldInfo produces an error of 'Input data required on port InputField' afterwards though. I thought this error is also related to SetFieldData not properly creating the new field.

dcwhite commented 8 years ago

@jessdtate Could you enumerate the various combinations of valid inputs and sizes, and test each one to see if they behave as expected? This task would be good to delegate to another student. Just @ them here and I'll assign it.

jessdtate commented 8 years ago

What a poorly written issue. I will go back and test this again. SCIrun has a thing where if there is an error in the module, it will disappear if there is an execute command, but no execution (no change in inputs). It could have been a normal error that I didn't see.

jessdtate commented 8 years ago

So the possible combinations are: num nodes x 1 -- scalar on nodes num elem x 1 -- scalar on elems num nodes x 3 -- vectors on nodes num elem x 3 -- vectors on elems num nodes x 6 -- tensors on nodes num elem x 6 -- tensors on elems num nodes x 9 -- tensors on nodes num elem x 9 -- tensors on elems

jessdtate commented 8 years ago

I have tried all those combos on a few fields, and I cannot replicate the error. However, I noticed that when trying to set tensors, the data doesn't set properly (tensor of zeros). should I make a new issue for that or post the example here?

jessdtate commented 8 years ago

Set field data is producing error messages when the fields didn't match. in every case that I tried.

dcwhite commented 8 years ago

There are also cases in the code where if a single value (scalar/vector/tensor) is readable, it's applied to all nodes/elems. We could remove that case. Also, what happens if num nodes == num elems?

dcwhite commented 8 years ago

And yes, just post the example here.

jessdtate commented 8 years ago

Here is the example net that will show the tensor from setfielddata (set to zero) and the correct mapping using ConvertIndicesToFieldData. test_setfielddata.srn5.zip

jessdtate commented 8 years ago

I forgot about a single value. That should stay, and the data location should be the previously assigned locations (if no definition, I think nodes is default). The case of num nodes==num elems is very rare, so It's harder to make a test case, but I think it would do the same. I will try to make a test case for that.

jessdtate commented 8 years ago

for num nodes == num elem, it looks like the code checks for the existing field basis, and uses node centering if not found. I still need to test it.

johaenns commented 8 years ago

SetFieldData also seems to only return a null field when the input is a sparse matrix though the dimension of field and matrix are fitting. It worked after I added a ConvertMatrixType before. I would assume this behavior is not intentional, since in most cases explicit conversions aren't necessary?

Tested on v5.0-beta.C, opensuse.

jessdtate commented 8 years ago

After some testing, especially at IBBM, I agree that the error message only returns null without an error when the matrix is sparse. There should be an exception added, even if it is planned to support sparse matrices in the future.

dcwhite commented 8 years ago

@darrellswenson Please update this issue with your specific problem. @jessdtate can help.

jessdtate commented 8 years ago

SelectSubMatrix has a similar problem, in that it will return a null output without showing an error. I don't think it is related to sparse matrices though. I will make another issue when I have time.

dcwhite commented 6 years ago

@jessdtate will clarify what needs to be fixed with a checklist

github-actions[bot] commented 5 years ago

Stale issue message

dcwhite commented 5 years ago

@jessdtate will clarify what needs to be fixed with a checklist

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 240 days with no activity. Remove the stale label or comment, or this will be closed in 60 days.

jessdtate commented 3 years ago
github-actions[bot] commented 2 years ago

This issue is stale because it has been open 240 days with no activity. Remove the stale label or comment, or this will be closed in 60 days.