-
Notifications
You must be signed in to change notification settings - Fork 0
Suggested edits to address reviews of Chris' density branch #48
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
Conversation
…d cell_H modifications
…of this workflow and remove old density schematic
chantelleleveille
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :) Thanks for doing that!
cfrick13
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may want to keep the test_voronoi.py file. This was not added by me. I think it is one of the tests written to validate the voronoi tesslation distance measurment which is useful for the toymodel.
Purpose
I took a stab at addressing the recommendations @chantelleleveille and I made in @cfrick13 's density branch PR. I am making a PR into his original branch with the hopes of having handled most of the edits needed to clear bandwidth for Chris. I completed this in a separate branch so that if I misinterpreted what Chris was trying to do or he doesn't like my edits is easy to change before affecting his branch.
Edits
figure_3_s4_workflow, so they are all one workflowTesting
figure_3_s4_workflowfirst - which now includes all workflows related to these figures - which succeededrun_all_manuscript_workflowsWhat didn't I address here that was part of the original density PR?
I didn't touch removing the actual old denisty features themselves, just the references to them. There were also a huge number of new features added in the
density_fig_updatesPR and I didn't feel prepared to decide how to pate that down. So @cfrick13 that will still need to be done with updates to your PR not covered here.