compdyn / partmc

Particle-resolved stochastic atmospheric aerosol model
http://lagrange.mechse.illinois.edu/partmc/
GNU General Public License v2.0
27 stars 15 forks source link

Monarch_1 test multi-cell not working #125

Closed cguzman95 closed 4 years ago

cguzman95 commented 4 years ago

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

mattldawson commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

cguzman95 commented 4 years ago

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

cguzman95 commented 4 years ago

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)