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

WIP: Hdf5 for particle dumping #76

Closed
wants to merge 79 commits into from
Closed

WIP: Hdf5 for particle dumping #76

wants to merge 79 commits into from

Conversation

rfbird
Copy link
Contributor

@rfbird rfbird commented Oct 10, 2019

No description provided.

Dave Nystrom and others added 30 commits December 30, 2018 16:39
…NL ATS-1 and CTS-1 machines.

Document how to use these two scripts.
…ment

starts out as the Cray default of PrgEnv-intel. This change checks for the
case where the user has modified their module environment and swaps it back
to the case assumed by the build script.
…le sort

implementation. Add build script support for a few more CMake variables that
were missing and should be availble to users of the build scripts.
@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #76 into devel will decrease coverage by 18.56%.
The diff coverage is 62.5%.

Impacted file tree graph

@@             Coverage Diff             @@
##            devel      #76       +/-   ##
===========================================
- Coverage   82.37%   63.81%   -18.57%     
===========================================
  Files         114      115        +1     
  Lines        6904    10528     +3624     
  Branches     1074     1495      +421     
===========================================
+ Hits         5687     6718     +1031     
- Misses        768     3273     +2505     
- Partials      449      537       +88
Impacted Files Coverage Δ
src/vpic/vpic.h 87.01% <ø> (+0.34%) ⬆️
src/vpic/vpic.cc 100% <100%> (ø) ⬆️
src/vpic/dump.cc 67.08% <100%> (+0.08%) ⬆️
src/grid/partition.cc 38.96% <50%> (+2.47%) ⬆️
src/util/pipelines/pipelines_thread.cc 60.74% <0%> (-0.94%) ⬇️
test/include/catch.hpp 28.35% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4832c7...29da114. Read the comment docs.

Copy link
Contributor

@dnystrom1 dnystrom1 left a comment

Choose a reason for hiding this comment

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

Howdy Bob,
Saw this and got worried. Are you adding the capability to have a global particle index while also retaining the local p->i index in the particle struct? I hope so. BTW, is this the right way to comment on a GitHub commit? Seems a bit different from the past scenario where you have requested me to review pull requests, etc.
Thanks,
Dave

@rfbird
Copy link
Contributor Author

rfbird commented Oct 16, 2019

Howdy Bob,
Saw this and got worried. Are you adding the capability to have a global particle index while also retaining the local p->i index in the particle struct? I hope so. BTW, is this the right way to comment on a GitHub commit? Seems a bit different from the past scenario where you have requested me to review pull requests, etc.
Thanks,
Dave

@dnystrom1 yea this is a fine way to comment on github, especially if you want to specifically address my work to extend the file IO to support HDF5

I have two branches currently:

This one -- where comments relating to "global id" are just so when we write the value of p->i (the particles cell) it's unique in global space

global_particle_id -- that adds a separate SoA that stores a globally unique id per particle

It sounds like you're referring to the second above, got but got scared by comment referring to the (unrelated) first?

Neither change at all how p->i functions and can be used

@dnystrom1
Copy link
Contributor

OK. Thanks. Good to know.

@rfbird
Copy link
Contributor Author

rfbird commented Dec 4, 2019

Replace by #80

@rfbird rfbird closed this Dec 4, 2019
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.

2 participants