-
Notifications
You must be signed in to change notification settings - Fork 12
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
9.configure script update #37
Conversation
- add yaml validation - add click usage - include all source information for components in regrid rose-app (until rose is retired)
- configure script validates yaml and writes more information in rose-apps - edits yaml updated to feel more like previous experiment XMLS - schema.json updated to validate new yaml format
- accidentally added these files from a merge conflict that was fixed previously on branch
Note: the configUpdates folder is not needed and will be taken out. The reason it is still in there in because of something Ian and I were looking at in our refactoring task, but it is solely for my development purposes. |
configuration_paths: | ||
rose-suite: "/home/Dana.Singh/pp/ppyaml/fre-cli/fre/frepp/configUpdates/rose-suite-am5_c96L33_amip.conf" | ||
rose-remap: "/home/Dana.Singh/pp/ppyaml/fre-cli/fre/frepp/configUpdates/remap-rose-app.conf" | ||
rose-regrid: "/home/Dana.Singh/pp/ppyaml/fre-cli/fre/frepp/configUpdates/regrid-rose-app.conf" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There 3 files are the rewritten configurations, yes?
I think it would be better to have these paths passed to the fre pp configure
tool rather than in the pp.yaml proper.
Partly because these paths are not "first-class" pp configuration and more internal settings, and partly as someone might want to apply the same pp.yaml to multiple workflows using the same pp.yaml:
fre pp configure -y ~/my-stock-pp.yaml -e am5_r1i1p1 -p gfdl.ncrc5-intel -t prod-openmp
fre pp configure -y ~/my-stock-pp.yaml -e am5_test -p gfdl.ncrc5-intel -t prod-openmp
chunk96: &PP_AMIP_CHUNK96 "1yr" | ||
analysisStart: &ANA_AMIP_START "1980" | ||
analysisStop: &ANA_AMIP_STOP "1988" | ||
defxyInterp: &defaultxyInterp "180,288" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat... these are like FRE properties, user variables that can be reused later?
I see the value set there, e.g. 10yr
, but is the variable analysisLen
or ANA_AMIP_LEN
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! They're used later in the components section. It was slightly confusing, the variable is defined as analysisLen
in the yaml, but the value actually reused later, that saves the value the user sets, is the one with the &
rose-suite: | ||
refineDiag: | ||
datastager_script: "$(includeDir)/refineDiag/refineDiag_data_stager_globalAve.csh" | ||
atmoscmip6_script: "$(includeDir)/refineDiag/refineDiag_atmos_cmip6.csh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "atmoscmip6_script" part of the yaml schema or a user setting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! This was originally following the draft you had started in the issue (#9). There were 2 refineDiag scripts but both were called script
so I found that the latter was the only one found. Changing the names from script
just let's us have access to both scripts. atmoscmip6_script
would be part of the schema here though. Good catch, I should change it to be more general, like taking away the 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick clarifying question here, in the rose-suite.conf
, the variable REFINEDIAG_SCRIPTS
is sometimes defined (if do_refineDiag is True). Would that variable need to point to both of these scripts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. REFINEDIAG_SCRIPTS can be space-separated, and each filepath should be an absolute path to the script.
target: "prod-openmp" | ||
|
||
#regrid-xy and remap-pp-components information | ||
regrid-remap-info: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! The components
part down is perfect as is, clearly analogous to a Bronx XML and I think users will be able to understand it.
Can the regrid-remap-info
header be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can. I put that in there for more organization but I'm sure it would work with just components
as the header here.
} | ||
}, | ||
"analysisLen": {"type": "string"}, | ||
"chunk96": {"type": "string"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name chunk96
is part of the schema, here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the chunk96
was part of those user variables that could be reused later. This name could change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User variable names shouldn't be in the pp.yaml schema, should they? Can users name their variables whatever they like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well in the pp.yaml, in order for it to be validated correctly, I think it should be defined as something. I can change it to something like pp_amip_chunk
or something instead of chunk96
. The actual variable that can be reused throughout the pp.yaml, i.e. &PP_AMIP_CHUNK96
, can be called anything the user wants though, as long as it's consistent through the yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm still confused. is the variable called pp_amip_chunk
or PP_AMIP_CHUNK96
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're good! That wasn't a great explanation on my part at all. I was looking for this link to help.
https://docs.geoserver.org/2.21.x/en/user/styling/ysld/reference/variables.html
With reusable yaml variables, it has this structure:
define: &variable <value>
Where:
define = pp_amip_chunk
$variable = &PP_AMIP_CHUNK96
(PP_AMIP_CHUNK96
was what the variable was called in the XML, and the variable to be reused in the yaml later on in the components section)
<value> = "1yr"
To my understanding, I see the define
(whatever we call it) to mainly be used to validate with the schema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aha, thanks for that. I see. I agree that's just what we want.
Can't we have just one variable name, PP_AMIP_CHUNK96
that is user set? What does the lower case pp_amip_chunk
in?
This is the stock example:
define: &variable <value>
Here's our example:
define: %PP_AMIP_CHUNK96 '1yr'
And then PP_AMIP_CHUNK96 can be used in the yaml below as:
*PP_AMIP_CHUNK96
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dana, thank you so much for this. I can't follow all of it, and I think some of the pp.yaml could be simplified, but if mostly works now, I'm inclined to think we should merge it and begin to use and test it more. We can always adjust as we go.
Thanks for taking a look through this Chris! I attempted to test this before, to see if the configurations were actually good to run a workflow, but there were some error messages in the |
…nd `edits-template.yaml`
No description provided.