-
Notifications
You must be signed in to change notification settings - Fork 66
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 AGORA test problem: High-res ICs [Reviewed] #346
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Great. I only have some minor comments.
@BarryTC Your |
@BarryTC
An example of the download script: #!/bin/bash
LOCAL_FILENAME1="HI"
LOCAL_FILENAME2="CloudyData_UVB=HM2012.h5"
FILE_ID1="677e4757999605c485c8de92"
FILE_ID2="677ca211999605c485c8de6c"
# 1. download
curl https://hub.yt/api/v1/item/${FILE_ID1}/download -o "${LOCAL_FILENAME1}.tar.gz"
curl https://hub.yt/api/v1/item/${FILE_ID2}/download -o "${LOCAL_FILENAME2}"
# 2. unzip
tar xzvf ${LOCAL_FILENAME1}.tar.gz
mv ${LOCAL_FILENAME1}/*.dat ./
rmdir ${LOCAL_FILENAME1}
rm ${LOCAL_FILENAME1}.tar.gz See also PR #408. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChunYen-Chen @BarryTC
I think it is good to download all the data we need from our ytHub, as we don't need to worry about files moving or changing from the source.
However, since the data in this test problem is from others, I would suggest we provide the link or reference to the original sources in the README or comments in this script. In this way, we could let users know where to find the original data (and it also serves as an alternative way to download the data if our links fail).
For example, the CloudyData_UVB=HM2012.h5
is from Grackle.
Are the AGORA HI and LOW from this link: http://goo.gl/8JzbIJ ?
example/test_problem/Hydro/AGORA_IsolatedGalaxy/download_ic_high_res.sh
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BarryTC Thanks for the contribution! I've added some minor inline comments. Additionally:
- Fix the conflicts
- Merge the latest
main
branch, then test whether you can run Graklce with double precision and GAMER with single precision for AGORA successfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BarryTC Everything looks good. Thanks for the contribution!
with comments from ChunYen-Chen addressed