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 tester images with netcdf #5491

Merged
merged 2 commits into from
Nov 14, 2023

Conversation

gassmoeller
Copy link
Member

Follow up to #5487
Adress part of #5447.

The deal.II 9.4 tester is done, I am rebuilding 9.5 right now, I will retrigger the tests for this PR tomorrow when that is done.

Copy link
Member

@tjhei tjhei left a comment

Choose a reason for hiding this comment

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

Awesome, thank you.

@gassmoeller gassmoeller merged commit e705f4f into geodynamics:main Nov 14, 2023
5 checks passed
@gassmoeller gassmoeller deleted the update_testers_with_netcdf branch November 14, 2023 15:49
@tjhei
Copy link
Member

tjhei commented Nov 14, 2023

@gassmoeller I think we need to specify -D NETCDF_DIR=/opt/netcdf-4.7.4/ or set NETCDF_DIR env variable for this to work. What do you think is the better approach?

@gassmoeller
Copy link
Member Author

gassmoeller commented Nov 14, 2023

I am open for either. However, I am confused: If candi installs netcdf, it also adds $NETCDF_DIR environment variable in the enable.sh script, which should enable ASPECT to find it. Is the root problem that enable.sh is not executed when we startup the container? Wouldnt that be the best solution then?

Edit: This would also take care of the extra hoops we have to jump through for astyle and DEAL_II_DIR.

@tjhei
Copy link
Member

tjhei commented Nov 14, 2023

Is the root problem that enable.sh is not executed when we startup the container? Wouldnt that be the best solution then?

Correct. I don't know if there is an easy way to do this, though. Maybe you know better. What I did in the tjhei/candi images is something like this:
https://github.com/tjhei/candi/blob/0a06374f7b2a61920d9fa0d9ecdf1f3409707e8f/v92/Dockerfile.in#L23-L43

@gassmoeller
Copy link
Member Author

Good point. Let me incorporate the variable into the image. Rebuilding right now.

@gassmoeller
Copy link
Member Author

See #5495.

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

Successfully merging this pull request may close these issues.

2 participants