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

Added 2 new unit converstions #3

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Added 2 new unit converstions #3

wants to merge 4 commits into from

Conversation

siligam
Copy link
Collaborator

@siligam siligam commented May 22, 2024

Auto-unit converstions for recom variables CO2f and pCO2s are added.
The negative sign used in "mmolC/m2/d" -> "kg m-2 s-1" is to correct the sign change for into ocean or into atm.

    auto-unit converstions for recom variables CO2f and pCO2s are added.
The negative sign used in "mmolC/m2/d" -> "kg m-2 s-1" is to correct the sign change for into ocean or into atm.
lib/step.rb Outdated
Comment on lines 217 to 220
when ["mmolC/m2/d", "kg m-2 s-1"]
cmds << CDO_MULC_cmd.new(-1.390127e-10) # negative sign to correct into ocean or into atm
when ["uatm", "Pa"]
cmd << CDO_MULC_cmd.new(0.101325)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging can you please add comments how these factors are obtained? Maybe similar like I did here: https://notes.desy.de/F8hTyk3PRielZrVdZRtyaQ?both#Seamore-missing-features-FESOM1-and-2

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better yet, define and use a documented variable. I personally hate magic numbers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisdane Thank you for asking the question. You can find the detail explanation on how factor were derived at this link https://notes.desy.de/F8hTyk3PRielZrVdZRtyaQ?view#Seamore-missing-features-FESOM1-and-2 (please see section 1.1 and 1.2)
Additionally, there are notes on the negative sign used in this factor. Instead of combining the factor and the negative sign adjustment in auto-unit conversion section, deferring the negative sign adjustment as a separate action in on cmorization steps might be a better approach. Please let me know your thoughts on aspect: if this extra step is required or is it good enough leave things as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah.. now I get it, to change the commit message to include you comments from the hedgedoc. will do it rightway.

Copy link
Collaborator Author

@siligam siligam May 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the link to updated commit which includes notes on how the factors were obtained.

seamore/lib/step.rb

Lines 218 to 239 in edde2d7

when ["mmolC/m2/d", "kg m-2 s-1"]
# notes on how this factor was derived (by chrisdane "Christopher Danek")
# e.g. recom variable: CO2f => CMIP6_Omon.json: fgco2
# 1) mmolC -> molC: /1e3
# 2) molC -> gC: *12.0107
# 3) gC -> kgC: /1e3
# 4) d-1 -> s-1: /86400
# -> mult_factor = 1/1e3*12.0107/1e3/86400 # = 1.390127e-10
# In the CMIP6_Omon.json:fgco2 case, an additional sign change is necessary:
# >0: into ocean -> >0: into atm: *-1
# -> mult_factor = mult_factor*-1
cmds << CDO_MULC_cmd.new(-1.390127e-10) # negative sign to correct into ocean or into atm
when ["uatm", "Pa"]
# notes on how this factor was derived (by chrisdane "Christopher Danek")
# e.g. recom variable: pCO2s => CMIP6_Omon.json: spco2
# 1) µatm -> atm: /1e6
# 2) atm -> Pa: *1.01325e5 (1 atm = 1.01325e5 Pa)
# -> mult_factor = 1/1e6*1.01325e5 # = 0.101325
#
# these notes are available on the following link
# https://notes.desy.de/F8hTyk3PRielZrVdZRtyaQ?view#Seamore-missing-features-FESOM1-and-2
cmds << CDO_MULC_cmd.new(0.101325)

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

Successfully merging this pull request may close these issues.

3 participants