-
Notifications
You must be signed in to change notification settings - Fork 9
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
adding Adios2 io support #113
Conversation
seegyoung
commented
Sep 9, 2024
- support for serial adios2 file I/O (set Omega_h_USE_ADIOS2 ON)
- MPI only with single process at the moment
- testing with kokkos and coda not included
/runtests |
Test Result: failure (details) |
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.
Hi Seegyoung, from what I see so far it looks like things are on the right track.
However, it seems like there are some missing files in your commit. I don't see the Omega_h_adios2.hpp
file, or where the write_adios2
code is located.
Thank you @seegyoung. Looks good. If it isn't there already, and while partitioned mesh support is under development, we should add a check to the adios writer and reader to ensure that the mesh is serial. |
I will update based on all the input from both of you. |
@seegyoung I still don't see the definitions for |
Sorry about the trouble - missing files are added |
@seegyoung Would you please remove the |
OK I will do. |
Hi Seegyoung, did you have a chance to address the comments from Cameron and I? |
Yes, it was removed on September 17th. Thanks for checking with me. |
@seegyoung can you update this pull request with your changes? I do not see the requested updates. |
I am not good at GitHub so I don't know what to do to resolve the issue and what's the source of the issue. Can someone can do this for me please? |
If you have changes that are committed into your local copy of the branch (i.e., on the SCOREC filesystem) you can run 'git push' to upload them to github. This PR will automatically be updated. I'm available to discuss via WebEx if needed. Please send me an email and we can pick a time. |
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.
@seegyoung i have added additional comments. Please address them and update this pull request.
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.
Looks good. Thank you. A few comments are below.
Co-authored-by: Cameron Smith <[email protected]>
@seegyoung Thanks for the updates. It looks like the |
That sounds good. I will do so. |
@cwsmith does Omega_h have logging or print functions that we should be using to be XSDK compliant? I didn't find anything with a quick check. |
@jacobmerson Good question. Adios2 is not a part of the xSDK (https://xsdk.info/release-1-1-0/). I did a couple of quick searches through the adios2 docs (https://adios2.readthedocs.io/en/v2.10.2/) and didn't find anything obvious. |
target_include_directories(omega_h PRIVATE "${ADIOS2_INCLUDE_DIRS}") | ||
target_link_libraries(omega_h PUBLIC "${ADIOS2_LIBRARIES}") |
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.
@cwsmith @seegyoung should we be using the adios2 target here? Or, do we need to handle this with the library variables because of bob.cmake
?
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 Cameron can answer this
I just had one remaining question on the |