-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix how yaml model config options are update #219
Conversation
We treat config options from files (with sections) differently from replacement options provided in a dictionary in code.
This seems to be necessary because the yaml parser doesn't understand numpy scalars.
TestingWith these instructions, I was able to set up the manufactured solution test case for Omega following @hyungyukang's instructions modified here for Intel on Chrysalis:
|
@hyungyukang, could you let me know if this fixes the problem for you as well? |
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.
@xylar , thank you very much for your quick update! This PR fixes the problem I had on pm-cpu.
After setting up the manufactured solution test with the Omega-0 test model, I submitted a job and was able to get this convergence result:
![image](https://private-user-images.githubusercontent.com/47987430/356292187-1e9b3863-a2e2-4bdc-9ee2-06a9573b1ef2.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1Mjg3OTIsIm5iZiI6MTczOTUyODQ5MiwicGF0aCI6Ii80Nzk4NzQzMC8zNTYyOTIxODctMWU5YjM4NjMtYTJlMi00YmRjLTllZTItMDZhOTU3M2IxZWYyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDEwMjEzMlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWU1ZmRjOTUwMjRiODRhNGZjNTQ1NGJhNzI2NmNjMDAwZTg5NWE2NjlhMWE0Mzg5NzlhNTQ2ZmE3NGIyYzM0YjkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.izogFtLWLjFJ_OQlXCW_iTn7GRgIMKyMyavclRffSW8)
Note that the model options are hardcoded in the test model now, and time step size is fixed.
I'm approving this PR as the yaml generation problem has been fixed.
Thanks again for your work on this!
@hyungyukang, thanks for the quick review! Glad it worked for you, too! |
With this update, we treat config options from files (with sections) differently from replacement options provided in a dictionary in code.
This merge also explicitly converts some model config options to floats. This seems to be necessary because the yaml parser doesn't understand numpy scalars.
Checklist
Testing
comment in the PR documents testing used to verify the changes