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 Most Builds to VL+PLMC #364

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

bcaddy
Copy link
Collaborator

@bcaddy bcaddy commented Jan 18, 2024

Summary

Updated all builds, except cosmology, to use the Van Leer (VL) integrator and Piecewise Linear Method in the Characteristic variables (PLMC) reconstruction.

I'd like @helenarichie and @ojwg to take a look to make sure I'm not breaking any of their builds.

@bcaddy bcaddy requested review from helenarichie and ojwg January 18, 2024 18:44
Updated the default hydro build to use the Van Leer (VL) and PLMC build
options. Also, updated the test data for those new builds
Also updated test data
Updated template, basic_scalar, disk, dust, rot_proj, static_grav to
use PLMC and Van Leer integrator
Avoids issues with running clang-tidy in non-VL builds
@bcaddy bcaddy force-pushed the dev-plmcvlDefault branch from 5fbc2da to f240b3b Compare January 25, 2024 14:05
@evaneschneider
Copy link
Collaborator

Just to double-check -- were you able to confirm that the spherical collapse test is actually collapsing now? It was working when I initially added the system test, but when @evazlimen documented it last August it was not.

@bcaddy
Copy link
Collaborator Author

bcaddy commented Jan 25, 2024

I compared slices of the old and new fiducial data and there were qualitatively very similar; ie a big dot in the middle. I thought the old fiducial data was definitely correct but I’m not totally sure.

@evaneschneider
Copy link
Collaborator

Sounds good. If you could add a plot of the new slice data to the documented examples page, that would be great (ultimately we will need to do this for all the examples, but that's a longer-term project).

@bcaddy
Copy link
Collaborator Author

bcaddy commented Jan 25, 2024

Sure. I assume you want a higher resulting plot like 128^3 or 256^3 rather than the low res test version?

@evaneschneider
Copy link
Collaborator

Yes, I think specifically the plot that results from using the example parameter file (which is 256^3).

Copy link
Collaborator

@helenarichie helenarichie left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@evaneschneider evaneschneider merged commit 4b50c55 into cholla-hydro:dev Jan 26, 2024
9 checks passed
@bcaddy bcaddy deleted the dev-plmcvlDefault branch February 2, 2024 15:54
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.

3 participants