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

DM-19443: Extract and visualize the local and the spatial AL kernel solution coefficients #123

Merged
merged 2 commits into from Jul 18, 2019

Conversation

kgabor
Copy link
Member

@kgabor kgabor commented May 10, 2019

No description provided.

@mrawls mrawls changed the title DM-19443 Add kernel coefficients debug plots, class diagram and their description DM-19443: Extract and visualize the local and the spatial AL kernel solution coefficients May 10, 2019
Copy link
Contributor

@mrawls mrawls left a comment

Choose a reason for hiding this comment

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

Please take a look at the individual GitHub comments.

After reading the new AL_implementation document, I am honestly still not very clear on what is happening, but I am lacking concrete suggestions for what parts of it need clarification. Perhaps adding some bullets or steps (first this happens, then this, then this) could help? Or adding a figure or two from the new debugging code that illustrates some of the key concepts, like the basis functions and kernel solutions as they are used in a real image? Perhaps copying/quoting some of the text from the API docs and elaborating on it directly? Or adding an algorithmic AL overview and then referring back to it as you delve into different classes and functions in the codebase?

Bottom line, I feel like I should come away with a clearer understanding of how the AL portion of diffim works after reading this, but instead I feel overwhelmed and confused. Maybe that makes two of us 😆 I'm sorry I don't have a clearer suggestion of what to do to improve it. It is better to have this than nothing, but I would like to see at least some clarifying additions to the writeup before it is merged. What would have helped past-Gabor who hadn't done this ticket yet to understand it all?

python/lsst/ip/diffim/utils.py Outdated Show resolved Hide resolved
python/lsst/ip/diffim/utils.py Show resolved Hide resolved
python/lsst/ip/diffim/utils.py Outdated Show resolved Hide resolved
python/lsst/ip/diffim/makeKernelBasisList.py Outdated Show resolved Hide resolved
doc/lsst.ip.diffim/code_notes.rst Outdated Show resolved Hide resolved
doc/lsst.ip.diffim/AL_implementation.rst Outdated Show resolved Hide resolved
doc/lsst.ip.diffim/AL_implementation.rst Outdated Show resolved Hide resolved
doc/lsst.ip.diffim/AL_implementation.rst Outdated Show resolved Hide resolved
doc/lsst.ip.diffim/AL_implementation.rst Outdated Show resolved Hide resolved
doc/lsst.ip.diffim/AL_implementation.rst Outdated Show resolved Hide resolved
@kgabor kgabor force-pushed the tickets/DM-19443 branch 2 times, most recently from eaf25da to a744095 Compare May 31, 2019 00:32
Gabor Kovacs added 2 commits July 17, 2019 17:43
…olutions for debugging with debug.plotKernelCoefficients=True. Figure numbering fix in docs.
@kgabor kgabor merged commit d60ad06 into master Jul 18, 2019
@kgabor kgabor deleted the tickets/DM-19443 branch July 18, 2019 00:49
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.

None yet

2 participants