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

update MOAB version #19

Merged
merged 1 commit into from
Apr 2, 2024
Merged

update MOAB version #19

merged 1 commit into from
Apr 2, 2024

Conversation

gonuke
Copy link
Member

@gonuke gonuke commented Mar 31, 2024

This appears to work using the newest release of MOAB.

Note: it probably makes sense to update the HDF5 version being used, but rebuilding it for each CI will slow things down. The alternative gets us into more complicated CI space.

Fixes #16

@gonuke gonuke marked this pull request as ready for review March 31, 2024 21:42
@gonuke gonuke requested a review from pshriwise March 31, 2024 21:42
Copy link
Member

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury left a comment

Choose a reason for hiding this comment

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

I think it is good to go. And, while talking about updating HDF5, I think we can use h5py here as we do not need the development files, I suppose.

@gonuke
Copy link
Member Author

gonuke commented Apr 1, 2024

We need the development files to build MOAB

@ahnaf-tahmid-chowdhury
Copy link
Member

I see, HDF5 is an optional dependency of MOAB, but for our project, we require HDF5 support in MOAB. Currently, MOAB utilizes system libraries for this purpose. Since I have an active PR, I'm considering adding all the system libraries directly into MOAB itself and building the wheel package? This way, we may not need to separately install HDF5 as it will be included within MOAB.

Here's a breakdown of the pros and cons:

Pros: When installing PyMOAB, all necessary dependencies will be automatically installed.
Cons: Since these dependencies will be precompiled, we won't be able to tweak them. If modifications are needed, we'll have to build them from source.

Copy link
Member

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks @gonuke. I'm wondering about the motivation for updating the HDF5 version. To me, we're trying to test in this package that we can reliably interpret the DAGMC data via PyMOAB, not so much whether or not the information that PyMOAB provides via the MOAB interface consistent across HDF5 versions, which seems like the responsibility of testing in MOAB itself.

@pshriwise
Copy link
Member

Going to merge this for progress' sake. We can continue the discussion about updating the HDF5 version separately if needed.

@pshriwise pshriwise merged commit c362d76 into svalinn:main Apr 2, 2024
1 check passed
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.

Update to newer MOAB for CI
3 participants