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

Monarch_1 test multi-cell not working #125

Closed
cguzman95 opened this issue Oct 17, 2019 · 5 comments
Closed

Monarch_1 test multi-cell not working #125

cguzman95 opened this issue Oct 17, 2019 · 5 comments
Assignees
Labels

Comments

@cguzman95
Copy link
Collaborator

cguzman95 commented Oct 17, 2019

I just pull the recent changes from chem_mod and update my adapt_gpu branch. I tested now the multi-cell on monarch_1 and all the results are 0, so something is wrong. I see some new things, for example, the camp_state now has an array of env_states to save the differents temp and pressure... and two different constructors (one_cell and multi_cell). I don't know why it has two different constructors when a class should have only one constructor, and I'm not sure how it selects one constructor or the other during the initialization...

But anyway, it seems env_states for all the cells are not set, since printing the temps on update_env_state returns 0 for multi-cell (for one cell is correct). So, want you to fix it or should I fix it in my manner? I liked the old one except for the need of calling update_env_state in the tests, but well, I don't think it needs so much change.

@mattldawson

@cguzman95 cguzman95 added the bug label Oct 17, 2019
@cguzman95 cguzman95 added this to the stable chemistry module milestone Oct 17, 2019
@cguzman95 cguzman95 self-assigned this Oct 17, 2019
@mattldawson
Copy link
Collaborator

Hi @cguzman95 - I can take a look at this tomorrow (I am at a conference today). If you can fix it that’s great, but please don’t just change things back to how they were. There are actually reasons for these changes. As far as the constructors go, these are called overloaded constructors, and they’re fairly common (maybe not so much in fortran, lol) here’s a link describing how they’re used in c++ https://www.google.com/amp/s/www.geeksforgeeks.org/constructor-overloading-c/amp/

@cguzman95
Copy link
Collaborator Author

cguzman95 commented Oct 17, 2019

As you wish, I'm finishing #114 and will continue with #126 to finish mostly the GPU part for stable version. After these two, I can carefully deal with this studying the workflow again.

And yeah, I know about overloaded constructors, but I have never seen them in fortran and I have to discover how it works

@mattldawson
Copy link
Collaborator

cool - i can help more once i’m back too - it’s hard to do code snippets on my phone

@cguzman95
Copy link
Collaborator Author

Okay, seems fixed on #129, it was nothing, the update_state of multi_cells was updating the wrong cell

@cguzman95
Copy link
Collaborator Author

Fixed also cb05_big: I added a half-fix and a TODO message: Basically it miss a mechanism to update different rates of update_rata (I fixed it to update all the cells with the same rate for the moment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants