Skip to content
This repository has been archived by the owner on Aug 6, 2024. It is now read-only.

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 requested a review from CCAstro35 January 15, 2021 18:55
@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
@nstarman nstarman deleted the sample_stuff branch January 16, 2021 02:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants