search

svalinn / DAGMC

Direct Accelerated Geometry Monte Carlo Toolkit
https://svalinn.github.io/DAGMC
Other
96 stars 61 forks source link

Switch to default `FindMOAB.cmake` #942

Open gonuke opened 5 months ago

gonuke commented 5 months ago

We currently rely on a custom built FindMOAB.cmake script to inspect the system and set the right variables for a build. However, MOAB now publishes a their own version of this. We should test to see if we can use this version both to reduce our code burden and rely on updates to that as they emerge.

At the same time, we can confirm whether version string is set by this FindMOAB.cmake to support #788

ahnaf-tahmid-chowdhury commented 5 months ago

Yes, I have noticed that. PyNE is also using custom FindMOAB.

ahnaf-tahmid-chowdhury commented 5 months ago

While updating PyNE's FindMOAB.cmake, I noticed that the MOABConfig.cmake file does not contain the version number of MOAB. I also observed that there is a file named libMOAB.so.X.X.X, where X.X.X represents the version number, in the $MOAB_ROOT/lib folder. Would you like to use this file to extract the MOAB version?

gonuke commented 5 months ago

I think we need to make a PR to MOAB to ensure that tis does include the version number. It is defined in the MOAB CMakeFiles.txt. I don't think it's robust to rely on the library name

ahnaf-tahmid-chowdhury commented 4 months ago

A PR in on the way.

vijaysm commented 4 months ago

The typical way to decipher the version is to look at src/moab/MOABConfig.h which gets generated at configure time and installed along with other headers. I can modify FindMOAB.cmake to look at the header to get the version number out. I'll check the submitted PR to make sure it generates the details correctly; if not, I would recommend doing the above.

gonuke commented 4 months ago

@vijaysm - do you provide a FindMOAB.cmake that sets all the variables that are important? I'd happily replace ours with that, especially if it includes the version number.

vijaysm commented 4 months ago

The current one doesn't decipher the version from headers. I think the PR is fine if it makes the workflow simpler. It would be good to expose it in config files rather than just in the header.