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

Pupillometry best practices #453

Merged
merged 9 commits into from Sep 28, 2021
Merged

Pupillometry best practices #453

merged 9 commits into from Sep 28, 2021

Conversation

N-M-T
Copy link
Collaborator

@N-M-T N-M-T commented Aug 12, 2021

No description provided.

@N-M-T N-M-T requested a review from papr August 12, 2021 10:43
Copy link
Member

@willpatera willpatera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@N-M-T this is great so far. I have left a few comments and suggestions.

src/core/best-practices.md Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
@N-M-T N-M-T requested a review from papr August 25, 2021 15:40
@willpatera
Copy link
Member

@papr are we still waiting on changes here? What's the status of this long open PR?

Copy link
Contributor

@papr papr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willpatera I didn't have the pupil-docs PRs on the top of my priority list, which is why they have been left alone for a while. @N-M-T please checkout my comments. Let me know if you see need to discuss them further.

src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Outdated Show resolved Hide resolved
src/core/best-practices.md Show resolved Hide resolved
@willpatera
Copy link
Member

@N-M-T please check reviews/requests and then we can likely merge.

@N-M-T N-M-T requested a review from papr September 22, 2021 16:28

The [pye3d model](/developer/core/pye3d/#pye3d-pupil-detection) regularly updates to account for headset slippage. In certain situations, this can lead to incorrect pupil size
estimates:
1. Hard Slippage: The headset slips on the wearer, and a 2d pupil datum is generated **prior** to the 3d model adjusting
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Hard Slippage: The headset slips on the wearer, and a 2d pupil datum is generated **prior** to the 3d model adjusting
1. Large Headset Slippage: The headset slips on the wearer, and a 2d pupil datum is generated **prior** to the 3d model adjusting

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard is a strange word in this context IMO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spatial amount of slippage is one thing. The temporal aspect needs also to be considered. Large slippage within short time is more difficult to compensate for the model compared to slippage over a longer period of time.

Copy link
Collaborator Author

@N-M-T N-M-T Sep 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to emphasise that 'Actual Headset Slippage' can be a cause of erroneous pupil size estimates, in addition to 'Model Adjustment Error' (where no headset slippage occurs). I don't think further details of the kind of slippage are needed (just to keep things as concise as possible). Therefore I have changed from 'Hard' to 'Actual'

Copy link
Member

@willpatera willpatera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few more changes @N-M-T Please resolve and then @papr can merge.


The [pye3d model](/developer/core/pye3d/#pye3d-pupil-detection) regularly updates to account for headset slippage. In certain situations, this can lead to incorrect pupil size
estimates:
1. Hard Slippage: The headset slips on the wearer, and a 2d pupil datum is generated **prior** to the 3d model adjusting
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard is a strange word in this context IMO.

@willpatera willpatera merged commit 3335ba7 into pupil-labs:master Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants