-
Notifications
You must be signed in to change notification settings - Fork 16
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
Remove pack/unpack from micro_mg #9
Comments
Is there any packing/unpacking in PUMAS? Since micro_mg_cam.F90 is a CAM issue, I think most of it lies there. But I've assigned this to you. I will try to up your PUMAS privileges next. |
It may be as simple as removing micro_pumas_data.F90 from PUMAS. This module has the guts of the pack/unpack routines. As soon as my research indicated I'd be touching PUMAS, I opened the issue. |
Okay. Thanks for doing this. Let me know if you need any more permissions or anything on PUMAS. |
@Katetc and @andrewgettelman I believe that I have run into an issue with removing the pack/unpack which will require another change in PUMAS. I am happy to implement the change in the PR which I will be opening for this issue. Before I make this change, I wanted to run it by you both. The problem is that micro_mg_tend in micro_mg3_0 only has one variable for dimension, mgncol. This is used to describe the extent of the arrays and for allocating new arrays. Loops then happen over all of mgncol. This results in problems when in CAM ncol /=pcols. This behavior was fine in the pack/unpack realm, as mgncol was always the same size as the array sizes and the extent of useful data. In micro_mg1_0, which was developed before pack/unpack, ncol and pcols are both passed in and used appropriately. I believe that I need to modify micro_mg3_0 to include both a pcols and ncol. If you agree, please suggest what you want the names to be. I expect that mgncol probably is the name for ncol. |
Can you explain why we need both dimensions? I recall it has to do with the last chunk not being the same as the total length of the arrays. This seems very CAM specific. Is there a way to generalize the interface to pass the number of columns needed and have it vary across chunks, say by calling a routine that tells you the size of the arrays by chunk? Since this will all be removed when we get to the CCPP, it seems unwise now to add it to PUMAS if we can avoid it. Thanks, Andrew |
The issue is that the input/output arrays in You could fix this in the call from If you change the dummy argument array declarations to be Fortran 90, I think the problem goes away. |
Thanks Steve, "If you change the dummy argument array declarations to be Fortran 90, I think the problem goes away." So is this the solution? So how do we do that? |
I can make this change to the dummy array declarations in my PR, if you approve. |
Most of the work would be in just converting the dummy array declarations (all the lines with
becomes
This would also apply to subroutines such as This approach should not require any changes in how the code is called. |
As Steve said, these changes will be internal and not effect the calling sequence. Since I am testing the pack/unpack, the subsequent PR will contain tested code. |
Thanks! Then this all sounds fine. I assume from the thread that the fix is internal and does not suffer from the performance hit Steve mentioned with changing the call. It also looks like it does not require pcol and ncol to be in the micro_mg routines, but only mgncol. That's great. Thanks! |
Hi everybody, this all sounds fine to me. I'm just super excited to not have the packing in any more. I'm ok with (almost) any changes required to get rid of it. |
This was completed in cam6_3_057 |
@andrewgettelman - I believe that micro_pumas_data.F90 still needs to be removed from the PUMAS repository. |
Thanks @cacraigucar for letting me know. I'm reopening and adding @Katetc so we can remove this at the next checkin for PUMAS (which should be soon: see new issue on updates). Thanks! |
Completed. |
Remove the pack/unpack structures from PUMAS in conjunction with removing from CAM
The text was updated successfully, but these errors were encountered: