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

Fix package name #12

Merged
merged 1 commit into from
Jan 27, 2020
Merged

Fix package name #12

merged 1 commit into from
Jan 27, 2020

Conversation

charleskawczynski
Copy link
Member

@charleskawczynski charleskawczynski commented Jan 8, 2020

Hi @odunbar, @dburov190, and @agarbuno

We need to standardize the package name. Please check that these changes don't break any results/tests. If they don't, feel free to merge and we can push forward. Once this is configured, we can set up some automated CI, so that changes can be made more easily.

Note that I've also changed the tolerance of 1e-8 to sqrt(eps(Float64))

docs/src/index.md Outdated Show resolved Hide resolved
src/neki.jl Outdated Show resolved Hide resolved
src/neki.jl Outdated Show resolved Hide resolved
src/neki.jl Outdated Show resolved Hide resolved
src/neki.jl Outdated Show resolved Hide resolved

Peform an iteration of NEKI, returning a new set of proposed `θs`.
Perform an iteration of NEKI, returning a new set of proposed `θs`.

See eqs. (5.4a-b) from https://arxiv.org/abs/1903.08866.
Copy link

Choose a reason for hiding this comment

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

Update reference, @agarbuno?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we change this to https://arxiv.org/pdf/2001.03689.pdf?

test/neki.jl Outdated Show resolved Hide resolved
@ali-ramadhan
Copy link
Member

ali-ramadhan commented Jan 8, 2020

Not sure if the plan is to register this as a Julia package but the name CES.jl might get rejected as it goes against naming guidelines 1 and 4: https://julialang.github.io/Pkg.jl/v1/creating-packages/#Package-naming-guidelines-1

1 Avoid jargon. In particular, avoid acronyms unless there is minimal possibility of confusion.
4 Err on the side of clarity, even if clarity seems long-winded to you.

CalibrateEmulateSample.jl would be accepted automatically I suspect.

@charleskawczynski
Copy link
Member Author

Thanks @ali-ramadhan! Good point. We'll want to change the name, but I'm not sure what the final name should be. I'm probably not the person to decide on the name. This PR just makes the naming consistent because, before, CES and Solus were both used but in different places, and I couldn't configure any tests.

@charleskawczynski
Copy link
Member Author

Is CalibrateEmulateSample.jl too verbose? @agarbuno @dburov190 @odunbar

@tapios
Copy link

tapios commented Jan 8, 2020

‘CalibrateEmulateSample.jl’ is good. You can also use ‘EnsembleKalmanSampler’ for ‘EKS’.

@tapios
Copy link

tapios commented Jan 8, 2020

Please also change the 1e-8 to ‘sqrt(eps)’. Whatever small effect it has on results is not material. We should address such issues when we can. Github issues seem to linger for a long time before being acted upon.

@charleskawczynski
Copy link
Member Author

Closes #13

Copy link
Contributor

@dburov190 dburov190 left a comment

Choose a reason for hiding this comment

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

Seems like this does break a bit of my stuff (examples directory), hold off a little bit, I'll fix that soon and then we can merge.

examples/GPR/main.jl Show resolved Hide resolved
@charleskawczynski
Copy link
Member Author

Seems like this does break a bit of my stuff (examples directory), hold off a little bit, I'll fix that soon and then we can merge.

Let me know if there's anything I can help with.

@charleskawczynski
Copy link
Member Author

@dburov190 @odunbar any updates on merging this?

@odunbar odunbar merged commit 1d884e3 into master Jan 27, 2020
@charleskawczynski charleskawczynski deleted the ck/FixName branch January 30, 2020 16: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.

5 participants