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

Sampling Potentials Architecture #17

Merged
merged 50 commits into from
Jan 16, 2021
Merged

Sampling Potentials Architecture #17

merged 50 commits into from
Jan 16, 2021

Conversation

nstarman
Copy link
Collaborator

@nstarman nstarman commented Nov 20, 2020

Description

Before we start calculating residuals, we want a code-agnostic method of sampling from a potential.
This introduces a registry system for adding sampling classes.

PR Checklist

  • Check out the contributing guidelines and code of conduct
  • Check out the contributing workflow ( for a practical example click here )
  • Give a detailed description of the PR above.
  • Document changes in the CHANGES.rst file. See existing changelog for examples.
  • Add tests, if applicable, to ensure code coverage never decreases.
  • Make sure the docs are up to date, if applicable, particularly the docstrings and RST files in docs folder.
  • Ensure linear history by rebasing, when requested by the maintainer.

@nstarman nstarman added this to the v0.1 milestone Nov 20, 2020
@nstarman nstarman self-assigned this Nov 20, 2020
@codecov
Copy link

codecov bot commented Nov 20, 2020

Codecov Report

Merging #17 (7e6d18f) into master (c85053b) will decrease coverage by 8.94%.
The diff coverage is 90.27%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #17      +/-   ##
===========================================
- Coverage   100.00%   91.05%   -8.95%     
===========================================
  Files            2       14      +12     
  Lines           28      313     +285     
===========================================
+ Hits            28      285     +257     
- Misses           0       28      +28     
Impacted Files Coverage Δ
discO/data/__init__.py 100.00% <ø> (ø)
discO/plugin/agama/__init__.py 0.00% <0.00%> (ø)
discO/plugin/agama/sample.py 0.00% <0.00%> (ø)
discO/plugin/__init__.py 77.77% <77.77%> (ø)
discO/__init__.py 100.00% <100.00%> (ø)
discO/common.py 100.00% <100.00%> (ø)
discO/config.py 100.00% <100.00%> (ø)
discO/core/__init__.py 100.00% <100.00%> (ø)
discO/core/core.py 100.00% <100.00%> (ø)
discO/core/measurement.py 100.00% <100.00%> (ø)
... and 16 more

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 c85053b...7e6d18f. Read the comment docs.

@GalOrrery GalOrrery deleted a comment from pep8speaks Nov 20, 2020
@nstarman nstarman changed the title Sampling with Galpy and AGAMA Sampling Potentials Architecture Dec 25, 2020
architecture
add notebook
CI comments
update gitignore
example notebook

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
@nstarman
Copy link
Collaborator Author

nstarman commented Jan 9, 2021

@CCAstro35
I uses https://pre-commit.com to style the code. The installation and running instructions are in the link. The code styling is actually embedded in the code in the .pre-commit-config.yaml file, so running

cd path/to/discO
pre-commit run —all-files

will clean up any code and documentation

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
@nstarman
Copy link
Collaborator Author

nstarman commented Jan 15, 2021

@CCAstro35 , except for AGAMA stuff, the test suite works well.
I don't think it's with holding up this PR for that. I've added a bug issue for this [#21]

@nstarman
Copy link
Collaborator Author

The one thing I haven't resolved is whether Sampler.resample should return a SkyCoord with shape (niter, nsamp) or (nsamp, niter). The former makes iterating over realizations easy, the latter makes a bit more sense in terms of ordering of dimensions. I think I favor the latter, as the iteration can be achieved by taking the transpose — for sample in samples.T produces SkyCoords with shape (nsamp,).

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
@nstarman nstarman added this to In progress in Paper 1 via automation Jan 15, 2021
@nstarman nstarman moved this from In progress to Review in progress in Paper 1 Jan 15, 2021
@CCAstro35
Copy link
Collaborator

@nstarman I agree. (nsamp, niter) makes more intuitive sense to me dimensionally, but what you said about making realizations easier is an important point though. I can see either way, but the latter works with me.

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
@nstarman
Copy link
Collaborator Author

I got the measurement sampler working on shaped SkyCoord arrays. It all pipes beautifully�.

@nstarman nstarman merged commit 4f4a865 into master Jan 16, 2021
Paper 1 automation moved this from Review in progress to Done Jan 16, 2021
@nstarman nstarman deleted the sample_stuff branch January 16, 2021 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants