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

Remove need for downstream codes to have Eigen3 installed #799

Open
gonuke opened this issue Feb 20, 2022 · 6 comments · May be fixed by #800
Open

Remove need for downstream codes to have Eigen3 installed #799

gonuke opened this issue Feb 20, 2022 · 6 comments · May be fixed by #800

Comments

@gonuke
Copy link
Member

gonuke commented Feb 20, 2022

Describe the Bug

Currently, any downstream code that links to DAGMC must have Eigen3 installed even if it has no direct dependence on Eigen3. Eigen3 is a source-code-header only library, so should be compiled into DAGMC libraries and not necessary to build dependent projects. This problem exists because the DAGMC header is "polluted" with too many other headers.

To Reproduce

Try to build a downstream code, e.g. OpenMC (see discussion in #739), without having Eigen3 installed.

Expected Behavior

Build a downstream code that has no independent/direct dependency on Eigen3 without having Eigen3 installed.

@gonuke gonuke linked a pull request Feb 20, 2022 that will close this issue
@gonuke
Copy link
Member Author

gonuke commented Feb 20, 2022

This could be more challenging that originally anticipated. It might require changes in MOAB instead. DAGMC currently does require some MOAB headers to be included in DagMC.hpp because we do use knowledge of the class internals from those headers. :(

@ahnaf-tahmid-chowdhury
Copy link
Member

Is there a lightweight alternative to MOAB that could be used in place of it? Or, could we selectively include only the necessary headers from MOAB instead of building the entire library?

@ahnaf-tahmid-chowdhury
Copy link
Member

According to MOAB documentation, Eigen3 is an optional dependency for MOAB only if TempestRemap tools are not to be built. Are we using TempestRemap? If not, we can remove Eigen3.

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury linked a pull request Feb 15, 2024 that will close this issue
@gonuke
Copy link
Member Author

gonuke commented Feb 15, 2024

Eigen3 is the preferred solution. This issue is not about removing Eigen3 as a dependency. This issue is about changing how we use it in careful ways to avoid forcing all downstream codes to need direct access to Eigen3.

@ahnaf-tahmid-chowdhury
Copy link
Member

From what I understand, to address the issue of Eigen3 being included in header files of MOAB and DAGMC that are required by downstream apps, we need to follow these steps:

  • Analyze the headers: We should go through all the header files in MOAB and DAGMC that include Eigen3 headers. Identify which headers are including Eigen3 and determine whether Eigen3 is actually necessary for the functionality provided by those headers.

  • Move Eigen3 includes to source files: Once we've identified the headers where Eigen3 is not necessary, we can move the #include <Eigen3> (or maybe something similar) statements from those headers to the corresponding source files (.cpp files) where the Eigen3 functionality is actually used.

  • Use forward declarations: In cases where Eigen3 is used only as a data type (e.g., as a member variable or function argument), we can use forward declarations instead of including the full Eigen3 header. This can help reduce the dependency on Eigen3 in downstream apps.

  • Test: After making the changes, test the downstream apps to ensure that they still build and function correctly without requiring Eigen3 headers.

@gonuke
Copy link
Member Author

gonuke commented Feb 15, 2024

Probably need to start in MOAB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants