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

Correct handling of state vars in multiple instances of component #93

Closed
tclune opened this issue Feb 8, 2022 · 5 comments
Closed

Correct handling of state vars in multiple instances of component #93

tclune opened this issue Feb 8, 2022 · 5 comments
Assignees
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. enhancement New feature or request

Comments

@tclune
Copy link
Contributor

tclune commented Feb 8, 2022

From @amdasilva

The other issue is the variable names in the multiple-instances of a component, CA, in particular.

CAMASS*    | kg kg-1    | xyz | C   |       | Carbonaceous Aerosol Mass Mixing Ratio                
CACONC*    | kg m-3     | xyz | C   |       | Carbonaceous Aerosol Mass Concentration               
CAEXTCOEF* | m-1        | xyz | C   | size(self%wavelengths_profile) | Carbonaceous Aerosol Extinction Coefficient 
CASCACOEF* | m-1        | xyz | C   | size(self%wavelengths_profile) | Carbonaceous Aerosol Scattering Coefficient

In the case that the instance is called CA.oc, the first variable becomes CAMASSOC, while it should have been OCMASS for backward compatibility. Then long name is now the same for OC, BC and BR. Should not have been done this way, but instead

*MASS    | kg kg-1    | xyz | C   |       | * Carbonaceous Aerosol Mass Mixing Ratio                
*CONC    | kg m-3     | xyz | C   |       | * Carbonaceous Aerosol Mass Concentration               
*EXTCOEF | m-1        | xyz | C   | size(self%wavelengths_profile) | * Carbonaceous Aerosol Extinction Coefficient 
*SCACOEF | m-1        | xyz | C   | size(self%wavelengths_profile) | * Carbonaceous Aerosol Scattering Coefficient

Where * gets replaced by OC, BC, BR. Also he has code like this that is not used for anything.

    if (comp_name(1:5) == 'CA.oc') then
       GCsuffix = 'OC'
    else if (comp_name(1:5) == 'CA.bc') then
       GCsuffix = 'BC'
    else if (comp_name(1:5) == 'CA.br') then
       GCsuffix = 'BR'
    end if

As it turns out, the variables are unnecessarily different. Also, no provisions for multiple instances in the other components.

@tclune will make any needed corrections to the MAPL ACG for this. This ticket is solely about correcting the usage in GOCART gridcomps when those fixes are available.

@tclune tclune added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. enhancement New feature or request labels Feb 8, 2022
@weiyuan-jiang
Copy link
Contributor

Where is the original issue ?

@amdasilva
Copy link
Collaborator

@tclune Have the updates above been implemented in ACG?

@tclune
Copy link
Contributor Author

tclune commented Mar 18, 2022

Yes - but we failed to merge that PR into MAPL develop until last night. I'd forgotten that I was waiting to confirm that it worked in GOCART and then lost track. So it is now on the develop branch and @mathomp4 will be rolling out a MAPL minor release this morning.

@amdasilva
Copy link
Collaborator

@weiyuan-jiang Changes need to be made in GOCART2G state specs and all HISTORY in gcm app needs to adjusted to reflect the new variable mames. This will create a PR in gcm app.

@amdasilva
Copy link
Collaborator

This seems to have been done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants