-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add multi-grid cell solving to MUSICA API for MICM #192
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.
"type": "USER_DEFINED", | ||
"MUSICA name" : "reaction 2", | ||
"reactants": {"E": {"qty": 1}}, | ||
"products": {"F": {"yield": 1}} | ||
}, | ||
{ | ||
"type": "USER_DEFINED", | ||
"MUSICA name" : "reaction 1", | ||
"reactants": {"D": {"qty": 1}}, | ||
"products": {"E": {"yield": 1}} |
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 only concern with this is that USER_DEFINED
isn't technically part of the camp configuration. However, I do think this is incredibly useful. We could remove photolysis, first order loss, emission, from the technical specification of the configuration because they are all just logical versions of user defined and it allows us to not have to do funny things with music box for integrated reaction rates.
I guess there's nothing actionable with that paragraph; I'm just putting thoughts out there
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.
Do you think we should add USER_DEFINED
to the CAMP configuration? I agree, it might make sense to replace the photolysis, emissions and first-order loss with this one type.
Thanks! I added #156 to the issues this closes. I think it might be enough for #178 also. If you agree, I can include it in the list. |
Sounds good, I agree with you |
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.
Will update 162_euler_option branch with the changes to MicmSolve.
Adds the ability to solve multiple grid cells with a single call to the MICM solver. Includes updates to the Fortran and Python wrappers. Also, modifies the Chapman test conditions to avoid convergence failures.
closes #175
I don't really like the fact that these function calls accept pointers to 2D data for temperature, pressure, concentrations, etc. where the size and shape are assumed to be correct with no checks. I suggest that we create a separate issue to address this.