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

Cosmology System Test & Gravity Support in SystemTestRunner Class #390

Merged
merged 7 commits into from
Jun 27, 2024

Conversation

bcaddy
Copy link
Collaborator

@bcaddy bcaddy commented Apr 16, 2024

Summary

  • Added gravity support to the SystemTestRunner class
  • Added a cosmology system test. Thank you to @bvillasen for providing the initial conditions and parameter files and helping answer my questions about this test problem.
  • Added Dual Energy to the default cosmology build
  • Make PPMP default for cosmo builds per offline discussion with @evaneschneider and @bvillasen
  • Updated the combine_hydro_particles.py script to work with gravity files as well. Now named combine_test_files.py
  • Fixed a few typos

@evaneschneider
Copy link
Collaborator

Is this ready to be merged? There is still a conflict with the test data.

bcaddy added 4 commits June 14, 2024 16:57
Added support for merging gravity files as well as particle files for
use as fiducial data for tests. Renamed the resulting script from
`combine_hydro_particles.py` to `combine_test_files.py`
also, fixed a typo in the hydro make file.
@bcaddy
Copy link
Collaborator Author

bcaddy commented Jun 14, 2024

I fixed the conflict. I'm running into a weird bug with some tests with DE turned on failing. I'll let you know when I have that resolved

bcaddy added 2 commits June 17, 2024 12:01
and fix a typo in the cosmo test parameter file
Cosmology uses a different value of DE_ETA_1 than other builds which
changed some tests results. This fixes the tests to account for that.
@bcaddy
Copy link
Collaborator Author

bcaddy commented Jun 17, 2024

This is ready to merge.

The bug was due to cosmology using a different value of DE_ETA_1 which changed the pressure in some tests. I've added handling for this to those tests.

@evaneschneider
Copy link
Collaborator

Just so I understand, which tests exactly did you need to change? The change by several orders of magnitude in the pressure is surprising to me, but maybe it's because I don't understand what is being compared.

@evaneschneider evaneschneider merged commit 7e388e2 into cholla-hydro:dev Jun 27, 2024
10 checks passed
@bcaddy
Copy link
Collaborator Author

bcaddy commented Jun 27, 2024

tALLLoadCellPrimitive and tALLReconstructionLoadData. DE_ETA_1 is very different in cosmology builds so which leads to the difference.

@evaneschneider
Copy link
Collaborator

I'm still not understanding. Don't those tests just load in fidicuial data? So why is the data different? I understand that the eta value is different, but I'm not understanding what the test data represents.

@bcaddy
Copy link
Collaborator Author

bcaddy commented Jun 27, 2024

Both of the functions being tested call the "get pressure from DE" function which internally depends on the value of DE_ETA_1 and the difference in that is enough with this fiducial data to change how it's handled.

@evaneschneider
Copy link
Collaborator

I see, thank you.

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.

2 participants