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

Allow Double Down v1.1.0 Installation in Dockerfile #944

Merged
merged 7 commits into from
Feb 29, 2024
Merged

Conversation

ahnaf-tahmid-chowdhury
Copy link
Member

Trying to fix the broken Docker build

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury marked this pull request as ready for review February 22, 2024 16:19
@ahnaf-tahmid-chowdhury
Copy link
Member Author

Is it necessary to test every time our build, once with DD off and once with DD on? This may create extra runs every time.

Copy link
Member

@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.

Is there a reason to add a geant4 off version

@@ -30,6 +30,7 @@ jobs:
5.5.1,
]
geant4_version : [
off,
Copy link
Member

Choose a reason for hiding this comment

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

Not sure it's worth have an extra image without GEANT?

Copy link
Member Author

Choose a reason for hiding this comment

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

While updating action.yml, I have noticed we are not using Geant4 by default. Do you like to change the default?

Copy link
Member

@gonuke gonuke Feb 27, 2024

Choose a reason for hiding this comment

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

The treatment of GEANT and Double Down are a little different from each other.

  1. Testing with or without GEANT only demonstrates whether DAGMC will integrate successfully with GEANT as a transport code.

  2. Testing with or without Double Down modifies the underlying fundamental behavior of DAGMC with all transport codes.

A few conclusions of this:

A) It makes sense to have fewer images, i.e. none that are build without GEANT and without DD because we can test our build without them even if they are installed.

B) It makes sense to do the advisory testing with the minimal number of options turned on, ie. without GEANT even if it is installed in the image.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've made changes to streamline the build process and reduce the number of arguments as much as possible. But it seems, I've ended up creating extra Docker images. 🙂

I think we have two options:

  1. Add the extra arguments again and remove conditions from the Dockerfile.
  2. Consider setting GEANT4 and DD always on in action.yml.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know, which step to proceed. Or, any other suggestions?

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury linked an issue Feb 29, 2024 that may be closed by this pull request
gonuke
gonuke previously approved these changes Feb 29, 2024
@gonuke
Copy link
Member

gonuke commented Feb 29, 2024

Looks like there are merge conflicts now

@gonuke gonuke merged commit cf35d27 into develop Feb 29, 2024
19 checks 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.

Failiure to build DAGMC with Double Down
2 participants