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

More verbose naming #95

Merged
merged 7 commits into from
Jan 5, 2021
Merged

More verbose naming #95

merged 7 commits into from
Jan 5, 2021

Conversation

odunbar
Copy link
Collaborator

@odunbar odunbar commented Dec 23, 2020

Purpose

To introduce more verbose naming conventions for filenames, modules and structs in the interest of more verbose naming conventions, as raised in Issue #82

Content

Name changes:

  • GPEmulator to GaussianProcessEmulator and GPObj to GaussianProcess
  • EKP to EnsembleKalmanProcesses and EKObj to EnsembleKalmanProcess
  • MCMC to MarkovChainMonteCarlo and MCMCObj to MCMC
  • find_ek_step -> find_ekp_stepsize
  • extract_GP_tp -> get_training_points

Changes made throughout:

  • src
  • docs
  • tests
  • examples

Additional tests

It seems the current runtests for Utilities and EKP are below the required codecov standard, therefore I shall write some additional tests before the PR can be merged.

Coauthored with @ilopezgp

@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #95 (0b6b0e1) into master (f40f997) will increase coverage by 3.61%.
The diff coverage is 97.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   85.19%   88.80%   +3.61%     
==========================================
  Files           7        7              
  Lines         547      545       -2     
==========================================
+ Hits          466      484      +18     
+ Misses         81       61      -20     
Impacted Files Coverage Δ
src/CalibrateEmulateSample.jl 100.00% <ø> (ø)
src/EnsembleKalmanProcesses.jl 97.53% <97.43%> (ø)
src/GaussianProcessEmulator.jl 87.11% <100.00%> (ø)
src/MarkovChainMonteCarlo.jl 85.43% <100.00%> (ø)
src/Utilities.jl 100.00% <100.00%> (+22.22%) ⬆️

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 f40f997...0b6b0e1. Read the comment docs.

@ilopezgp
Copy link
Contributor

ilopezgp commented Jan 5, 2021

When running test/EnsembleKalmanProcesses/runtests.jl, it seems like TEST_PLOT_OUTPUT is not defined. Can you verify this behavior @odunbar ?

@@ -77,6 +82,7 @@ using CalibrateEmulateSample.ParameterDistributionStorage
# Plot evolution of the EKI particles
eki_final_result = vec(mean(ekiobj.u[end], dims=1))

TEST_PLOT_OUTPUT = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When running test/EnsembleKalmanProcesses/runtests.jl, it seems like TEST_PLOT_OUTPUT is not defined. Can you verify this behavior @odunbar ?

In theory we should run the tests as a block i.e from test/runtests.jl and so this is where TEST_PLOT_OUTPUT is defined (via a get method with 0 default if the CES_TEST_PLOT_OUTPUT variable is not defined). If you directly run the EKP block from test/EnsembleKalmanProcesses/runtests.jl you would need to define it.

So i would remove the line you've added here. Otherwise these run for me! Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edit: I've removed this in latest commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks!

@odunbar
Copy link
Collaborator Author

odunbar commented Jan 5, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 5, 2021

Build succeeded:

@bors bors bot merged commit e68f9ae into master Jan 5, 2021
@bors bors bot deleted the orad/verbose-names branch January 5, 2021 18:54
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