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

Wns fast #8

Merged
merged 4 commits into from Nov 22, 2017

Conversation

Projects
None yet
2 participants
@sextonw
Contributor

sextonw commented Nov 22, 2017

No description provided.

@larsvilhuber

This comment has been minimized.

Show comment
Hide comment
@larsvilhuber

larsvilhuber Nov 22, 2017

Member

@sextonw @wns32 : Could you adjust the pull request to use directory programs/ not newprograms/ ? this replaces the existing programs, not adds new programs. I can't do that during the pull request.

Member

larsvilhuber commented Nov 22, 2017

@sextonw @wns32 : Could you adjust the pull request to use directory programs/ not newprograms/ ? this replaces the existing programs, not adds new programs. I can't do that during the pull request.

@larsvilhuber larsvilhuber self-requested a review Nov 22, 2017

@sextonw

This comment has been minimized.

Show comment
Hide comment
@sextonw

sextonw Nov 22, 2017

Contributor

@larsvilhuber Yes, I'll take care of that.

Contributor

sextonw commented Nov 22, 2017

@larsvilhuber Yes, I'll take care of that.

@larsvilhuber

This comment has been minimized.

Show comment
Hide comment
@larsvilhuber

larsvilhuber Nov 22, 2017

Member
Member

larsvilhuber commented Nov 22, 2017

@sextonw

This comment has been minimized.

Show comment
Hide comment
@sextonw

sextonw Nov 22, 2017

Contributor

Ok. I think c29103f should do it. I'll update the readme soon to reflect the major differences in versions. The 2 main ones though are: 1) Recode of race vars was removed because it didn't meet the needs our collaborators. We may revisit this in the future but the recodes were only added for a specific use case that is no longer revelant because the race codes are changing on the 2020 census. 2) The 'id' var values have changed though the variable still serves the same purpose. It uniquely identifies replicated individuals with replicated households. Household 'ids' now have the form serialno.replicationno This change was necessary to facilitate a significant restructuring of the code. The big difference now is that rep_syn_housing and rep_syn_person can now be run simultaneously. Each now take about 3 hours to complete putting the total runtime of all programs around 3.5 hrs.

Contributor

sextonw commented Nov 22, 2017

Ok. I think c29103f should do it. I'll update the readme soon to reflect the major differences in versions. The 2 main ones though are: 1) Recode of race vars was removed because it didn't meet the needs our collaborators. We may revisit this in the future but the recodes were only added for a specific use case that is no longer revelant because the race codes are changing on the 2020 census. 2) The 'id' var values have changed though the variable still serves the same purpose. It uniquely identifies replicated individuals with replicated households. Household 'ids' now have the form serialno.replicationno This change was necessary to facilitate a significant restructuring of the code. The big difference now is that rep_syn_housing and rep_syn_person can now be run simultaneously. Each now take about 3 hours to complete putting the total runtime of all programs around 3.5 hrs.

@sextonw

This comment has been minimized.

Show comment
Hide comment
@sextonw

sextonw Nov 22, 2017

Contributor

I'm going offline now.

Contributor

sextonw commented Nov 22, 2017

I'm going offline now.

@larsvilhuber

This comment has been minimized.

Show comment
Hide comment
@larsvilhuber

larsvilhuber Nov 22, 2017

Member
Member

larsvilhuber commented Nov 22, 2017

@larsvilhuber

This comment has been minimized.

Show comment
Hide comment
@larsvilhuber

larsvilhuber Nov 22, 2017

Member

Looks good to me. The comments address all my questions. Updates to the README coming.

Member

larsvilhuber commented Nov 22, 2017

Looks good to me. The comments address all my questions. Updates to the README coming.

@larsvilhuber

Comments by @sextonw and @wns32 address all my questions.

@larsvilhuber larsvilhuber merged commit 5eb8c2e into master Nov 22, 2017

@larsvilhuber

This comment has been minimized.

Show comment
Hide comment
@larsvilhuber

larsvilhuber Nov 22, 2017

Member

@sextonw : when you have time, delete the branch wns-fast.

Member

larsvilhuber commented Nov 22, 2017

@sextonw : when you have time, delete the branch wns-fast.

@sextonw sextonw deleted the wns-fast branch Nov 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment