Skip to content
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

loadC3D in pyCGM_Single/pyCGMIO.py does not reset the mydictunlabeled dictionary. #35

Open
niravkin opened this issue Oct 24, 2020 · 5 comments · Fixed by #36
Open

loadC3D in pyCGM_Single/pyCGMIO.py does not reset the mydictunlabeled dictionary. #35

niravkin opened this issue Oct 24, 2020 · 5 comments · Fixed by #36
Assignees
Labels

Comments

@niravkin
Copy link
Contributor

for frame_no, points, analog in reader.read_frames(True,True):

In the above loop, the dictionary mydict is set to an empty dictionary to prepare it for the next frame in the loop, but the dictionary mydictunlabeled is not. This causes unlabeled data to load incorrectly when using loadC3D.

@cadop
Copy link
Owner

cadop commented Oct 26, 2020

Can you clarify what the problem is? What is being not loaded correctly?

@niravkin
Copy link
Contributor Author

When I use loadC3D without resetting the mydictunlabeled dictionary first, the values for unlabeled frames are not correct. If I change the function to not split the data into labeled and unlabeled data, I can get accurately loaded data for all frames. I compared this to the output I get from splitting the data into the two dictionaries, and the data for the labeled dictionaries matches, but the data for the unlabeled dictionary does not. The problem is fixed when I add this line at the end of the for loop: mydictunlabeled = {}

@cadop
Copy link
Owner

cadop commented Oct 26, 2020

Can you post a snippet of the input and output/expected output. I see the problem you are saying, but want to make sure we didn't have a reason to do that.

@niravkin
Copy link
Contributor Author

Here is a snippet of the code I am using for testing this bug. loadC3D is the function as it exists right now. loadC3DModified is the function where the data and unlabeled data dictionaries are combined, and loadC3DFixed is the function with the proposed change. I am using Sample_Static.c3d for testing. loadC3D returns data in the format: [data,dataunlabeled,markers], so I access index 0 in modifiedc3dresults since I have combined data and unlabeled data. I am testing for the value for the unlabeled data with the name *112 in frame 0.

filename = 'Sample_Static.c3d'
modifiedc3dresults = loadC3DModified(filename) #combined data and unlabeled
oldc3dresults = loadC3D(filename)
newc3dresults = loadC3DFixed(filename)
print('Actual c3d file value:', modifiedc3dresults[0][0]['*112'])
print('Old loadC3D value:', oldc3dresults[1][0]['*112'])
print('Fixed loadC3D value:', newc3dresults[1][0]['*112'])

It produces this output:
Actual c3d file value: [-225.52651978 405.57913208 1214.45861816]
Old loadC3D value: [-225.48298645 400.69299316 1209.33276367]
Fixed loadC3D value: [-225.52651978 405.57913208 1214.45861816]

@cadop cadop added the bug label Oct 26, 2020
@cadop
Copy link
Owner

cadop commented Oct 26, 2020

Okay now I see it. Good find. I suspect the issue is the dictionary added to the list is maintaining a reference to it and is being modified each loop, which modifies an earlier value.

Please make a PR with your fix and label it to close this issue.

@cadop cadop linked a pull request Oct 27, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants