-
Notifications
You must be signed in to change notification settings - Fork 12
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
Test musica #225
Test musica #225
Conversation
@K20shores I think I know what is going on with the failed test. Would it be alright if pushed the fix to your branch? |
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.
My limited 2 cents worth review
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.
Submitting this time with my comment
better language
Co-authored-by: Jesse Nusbaumer <[email protected]>
I see that there is a netCDF file being included with this PR. Typically we have prohibited files from being committed to the code repos and instead, files reside in inputdata. Can the testing files reside in inputdata and be downloaded in the typical cime way? |
@cacraigucar after #222 is merged, into development and then this branch the binary files will be removed. in #222, I added FTP downloads for the netcdf files and I based this branch off of that work |
@nusbaume Now that the other PR is merged, would you like me to copy this dockerfile into the |
@K20shores that sounds good, unless you want to move both files to under the |
@nusbaume Done. I moved the dockerfiles into the docker folder and updated instructions accordingly |
Just an FYI to folks (I'm not sure where the proper place is to document this). During the manage_externals/checkout_externals step, I got the following message: => ERROR [ 9/21] RUN ./manage_externals/checkout_externals 0.5s
@mwaxmonsky helped me and he finally figured out that my TortoiseGit (which I used to do my git clone/checkout on my Windows box) had an option to automatically convert line feeds into Windows line feeds. I believe this is the default setting for TortoiseGit (but my admins installed this years ago, so I'm not completely sure). Turning off the auto convert, allowed me to proceed with the manage_externals/checkout_externals step. |
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.
From the usability standpoint, I approve this PR
@K20shores I have some changes to |
yes, please push directly to this branch! |
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.
I've pushed the changes I wanted, so this PR is good on my end!
I should note that I tested my changes by building/running both the cam-sima
and musica
containers. Still, it would be good to have others re-build/run the containers themselves at least one more time to make sure I didn't break anything. Thanks!
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 good! thanks!
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 good to me! Apologies for the delay!
Builds CAM-SIMA with musica. If you look at the output of the cesm log and search of "new state", you'll see the result of print statements we have from the micm component. Again, not much happens as of now with the component, but the connections are there