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

[WIP] pilot parameter refactor #62

Merged
merged 1 commit into from
May 18, 2022
Merged

[WIP] pilot parameter refactor #62

merged 1 commit into from
May 18, 2022

Conversation

odunbar
Copy link
Contributor

@odunbar odunbar commented Feb 10, 2022

Purpose of this PR

Relating to issues #57
Provides a toml format for other packages to use. Currently see

  • PR 83 on Clima/Thermodynamics
  • PR 55 on Clima/CloiudMicrophysics

Also we can now write re-readable log files

In this PR

  • Documentation! See the latest documentor build below.
  • New wrapper ParamDict{FT}around dictionary read from toml. Currently parametric on one type of all the values, provided when it is built (default is Float64). Construction:
create_parameter_struct(override_filepath, default_filepath; dict_type="alias",value_type=Float64) # builds with override and custom default provided
create_parameter_struct(override_filepath ; dict_type="alias",value_type=Float64) # builds, overriding the default from CLIMAParameters
create_parameter_struct(; dict_type="alias",value_type=Float64) # builds using only default in CLIMAParameters
  • Automated parameter logging on reading parameter values: (can do multiple parameters at once). At the call of this function, the type is enforced on the return values.
get_parameter_values!(pd::ParamDict, names, component; log_component=true) #logs component in pd
get_parameter_values(pd::ParamDict,names) # no logging in pd
  • writing the re-readable log file in log_parameter_information(pd::ParamDict,filepath). Example parameter tested in CloudMicrophysics.jl:
# initial parameter file
[molar_mass_water]
alias = "molmass_water"
value = 0.01801528
type = "float"

In this test, a copy of the parameters was created and stored in (1) Thermodynamics, (2) the general CloudMicrophysics, and (3) the specific 1-moment-based Microphysics scheme. Indeed the logfile shows this information below

# log file
[molar_mass_water]
alias = "molmass_water"
value = 0.01801528
type = "float"
used_in = ["Thermodynamics", "CloudMicrophysicsParameters", "Microphysics_1M_Parameters"]

The log_parameter_information(pd::ParamDict,filepath; warn_else_error="warn") will also throw warnings if a parameter is in the override file, but unused in the simulation. This works as parameters from the override file are checked for whether they have gained a "used_in" tag, if not found then an this is logged as a warning or error

TODO

  • still using alias based structs
  • still based on float, and array-of-float parameters

@codecov
Copy link

codecov bot commented Feb 10, 2022

Codecov Report

Merging #62 (6370511) into main (bacec5a) will increase coverage by 12.32%.
The diff coverage is 94.33%.

❗ Current head 6370511 differs from pull request most recent head b3839b5. Consider uploading reports for the commit b3839b5 to get more accurate results

@@             Coverage Diff             @@
##             main      #62       +/-   ##
===========================================
+ Coverage   83.72%   96.04%   +12.32%     
===========================================
  Files           5        6        +1     
  Lines         172      278      +106     
===========================================
+ Hits          144      267      +123     
+ Misses         28       11       -17     
Impacted Files Coverage Δ
src/file_parsing.jl 94.33% <94.33%> (ø)
src/Planet/planet_parameters.jl 100.00% <0.00%> (+35.29%) ⬆️
src/UniversalConstants.jl 100.00% <0.00%> (+71.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bacec5a...b3839b5. Read the comment docs.

Project.toml Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
@charleskawczynski
Copy link
Member

Can we also add an example of what this will look like in practice? For example:

struct ParameterBox{FT}
alpha::FT
beta::FT
end

# construct / extract / use one

@odunbar
Copy link
Contributor Author

odunbar commented May 17, 2022

I have docs pages for this already (https://clima.github.io/CLIMAParameters.jl/previews/PR63/parameter_structs/)
but i we could add something into the tests

@charleskawczynski charleskawczynski force-pushed the orad/refactor-parameters-v2 branch 8 times, most recently from 91e482e to e11015c Compare May 18, 2022 02:11
bors bot added a commit that referenced this pull request May 18, 2022
75: Add and apply JuliaFormatter r=charleskawczynski a=charleskawczynski

Now that this revamp is getting near, and it involves code rather than many functions that just return constants, it makes sense to add the formatter. This PR adds and applies the JuliaFormatter.

This should barely have any conflicts with #62, maybe a few small ones in the docs.

Co-authored-by: Charles Kawczynski <[email protected]>
@charleskawczynski charleskawczynski force-pushed the orad/refactor-parameters-v2 branch 2 times, most recently from 7299a66 to c121b0a Compare May 18, 2022 22:55
Enable get_parameter_values to deal with array-valued parameters

- `get_parameter_values` extracts the requested parameter values and returns
  them as an array. The returned array contains the values corresponding
  to the given list of parameter names (unless a single scalar  parameter is
  requested, in which case the function returns a float), regardless of whether
  these parameters are all of the same type.
- E.g., given a list of parameter names ["scalar_param", "array_param"] with
  `scalar_param` having a scalar value and `array_param` being array-valued,
  the first element of the returned array will be a float, and the second
  element will be an array
- Added more tests to `toml_consistency.jl` to check if parsing, extracting,
  and writing of array-valued parameters works. Also added a test for
  `merge_override_default_values`

Enforce types and move `array_parameters.toml` to test folder

Add V0 of TOML file parsing for UQ parameters

Add functionality to write parameter ensembles to toml files

Move docstrings above functions

Add improvements and updates

- save_parameter_ensemble now creates a separate subdirectory for
  each ensemble member, while the name of the saved parameter file
  after each ensemble Kalman update is the same for all members.
  Example:
    iteration_01/member_01/test_parameters.toml
    iteration_01/member_02/test_parameters.toml
    etc
- correct saving of multidimensional parameters using parameter slices

Move dict version of `write_log_file` from file_parsing.jl to file_parsing_uq.jl

Within the CES API, `write_log_file` will take a parameter dictionary as input
(rather than a parameter struct containing a parameter dictionary, as in the
running-the-climate-model API). Since the two APIs will go separate ways in the
future, the CES version of `write_log_file` should be located in
`file_parsing_uq.jl`.

added typo-checks for overrides, provides warnings or error on parameter logs

slightly more revealing test

documentation and guides

Modify parsing of UQ parameters to work with new `ParameterDistribution` API

Make tests work

Add regularization flags

Revise based on Ollie's comments, and remove print commands from tests

moved toml and old files into directories

parameter box example (as test set)

Improve create_parameter_struct interface

warn_else_error -> warn_or_error, and docs update

Fix some docs, minor adjustments

Apply formatter

WIP

Refactor
Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

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

YESSS

@odunbar
Copy link
Contributor Author

odunbar commented May 18, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented May 18, 2022

🔒 Permission denied

Existing reviewers: click here to make odunbar a reviewer

@odunbar
Copy link
Contributor Author

odunbar commented May 18, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented May 18, 2022

@bors bors bot merged commit 5714fa8 into main May 18, 2022
@bors bors bot deleted the orad/refactor-parameters-v2 branch May 18, 2022 23:12
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.

2 participants