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

add option to build with or without doubledown #30

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

bquan0
Copy link

@bquan0 bquan0 commented Sep 27, 2024

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

I added a variable in conda_build_config.yaml for doubledown and modified build.sh and meta.yaml to allow packages to be built with or without doubledown.

Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

@bquan0
Copy link
Author

bquan0 commented Sep 27, 2024

@conda-forge-admin, please rerender

Copy link

Hi! This is the friendly automated conda-forge-linting service.

I Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint --conda-forge . from the recipe directory.

@bquan0
Copy link
Author

bquan0 commented Sep 27, 2024

@conda-forge-admin, please rerender

Copy link

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest webservices GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or you can try rerendering locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/dagmc-feedstock/actions/runs/11077696903.

Copy link

github-actions bot commented Sep 27, 2024

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

@shimwell
Copy link
Contributor

It would be great to include doubledown

I thought it was not available on conda so that might need fixing first.

Here is the related issue

pshriwise/double-down#31

Copy link
Contributor

@gonuke gonuke 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 needs more help to find double down

@@ -31,7 +48,6 @@ cmake -DBUILD_MCNP5=OFF \
-DBUILD_STATIC_EXE=OFF \
-DBUILD_PIC=OFF \
-DBUILD_RPATH=ON \
-DDOUBLE_DOWN=OFF \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to specify the location of double down:

        -Ddd_ROOT=${PREFIX} && \

@bquan0
Copy link
Author

bquan0 commented Oct 5, 2024

@conda-forge-admin, please rerender

@bquan0
Copy link
Author

bquan0 commented Oct 5, 2024

After specifying the location of double down and rerendering the feedstock since I changed the name of the double down variable to dd, all tests are now passing.

@bquan0 bquan0 requested a review from gonuke October 5, 2024 22:30
@gonuke
Copy link
Contributor

gonuke commented Oct 5, 2024

When I look at the tests, I don't see any evidence that this was built with double down. I think it's because it's using v3.2.3 and the CMake configuration is out of date relative to develop. Maybe we need to issue another release of DAGMC.

@pshriwise
Copy link
Contributor

@gonuke, @bquan0 and I touched base this morning. As a short term solution I suggested verbose output from one of the tests which will show whether or not double down is enabled in the output:

1: Using the DOUBLE-DOWN interface to Embree.
1: Loading file test_dagmc_impl.h5m
1: Initializing the GeomQueryTool...
1: Using faceting tolerance: 0.001
1: Building acceleration data structures...

In the long term, for partial verification we certainly should add some output from DAGMC that double down is enabled during the configuration step.

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.

4 participants