Diffusion Maps rebase, new PR for final review #894

jdetle · 2016-07-07T20:29:26Z

Fixes #857
I am moving this here because I don't want to overwrite my old diffusion branch just in case the rebasing is deemed overkill and it will make it easier for @richardjgowers @kain88-de to help in reviewing this.

Changes made in this Pull Request:

Added a Diffusion Map module for non-linear dimension reduction

Todo:

Package import for mdanalysis
Document kwargs for dmap
Fix sphinx linking
Change warning to 5000 frames
Document API for metric
Change license to Gnu public license v2
Change docs to remove reference to fokker-plank
remove notebook until integrated repo
show how to plot data

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

style fixes, protection fixes, added docs fix of some broken areas fixed selections and updated tests accordingly switched order of imports [skip ci]

map calculation decided to calculate epsilon as a function outside of class for maximum usability. Change of direction in API, more work to be done implementing everything Changes to docs, functions, tried to make it very evident how we were following the lafon paper in terms of algorithm. added timescaling, fixed some parts

DistMatrix is its own AnalysisBases subclass, DiffusionMap still is an AnalysisBase subclass but only for the actual diffusion map embedding. Previous _conclude function renamed to kernel_decompose. Changed things around to get embedding working Added tests, found bugs, squashed bugs. Eliminated square of distance, random mistake

operational diffusion map

Eliminated spectral gap function... for now Fixed matrix multiplication bug Fixed save issue Better single_frame loop Name changes, styling updates and doc fixes.

Changed code further to make transform a simple iteration. Provided functionality for doing a diffusion map with just a distance matrix.

…s a scale parameter. Added exception to be thrown for large distance matrices. Updated diffusion map after some test-driven development, added error thrown for large frame size. Change to warning Removed exception test, fixed typo Fixed transform, kwargs acceptance, removed Epsilon after accidentally adding it back in. Minor fixes to docs. Fixes as suggested to be more pythonic and elegant Fixes to docs, Transform function.

unneccesarily exponentiating a diagonalizable matrix. Removed timescaling init, removed tests for anisotropic kernel and timescaling.

…p ci]

jdetle · 2016-07-07T20:46:40Z

@kain88-de, the only thing left to do that I see is to check that [Theobald2005]_ is properly linked when the documentation is built, that led me to fixing a problem with my sphinx building, but theobald is properly linked and the pages are building appropriately.

EDIT: Just checked and actually the paper linking isn't working so I have to fix that.

kain88-de · 2016-07-07T20:49:59Z

package/MDAnalysis/analysis/diffusionmap.py

+
+:Authors: Eugen Hruska, John Detlefs
+:Year: 2016
+:Copyright: GNU Public License v3


This isn't our default license that is problematic. @euhruska are you ok if we publish this under GPLv2.

coveralls · 2016-07-11T02:29:45Z

Coverage increased (+0.06%) to 81.073% when pulling b299d89 on jdetle:diffusion_rebase into e801d35 on MDAnalysis:develop.

jdetle · 2016-07-11T03:28:56Z

Don't merge this I realized the tests are incomplete/ kind of poor.

kain88-de · 2016-07-11T14:17:47Z

@jdetle can you say whats missing in the tests.

jdetle · 2016-07-11T16:38:09Z

In #889 I learned that we should be asserting against a reference value in
all tests and there is a warning I haven't covered. Just landed in Austin
but I'll finish the tests today and get everything done.
On Mon, Jul 11, 2016 at 9:17 AM Max Linke [email protected] wrote:

@jdetle https://github.com/jdetle can you say whats missing in the
tests.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#894 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AKcARtg3At1hQiC_3mQd_pb4n9UWDR_Xks5qUlCMgaJpZM4JHd_i
.

Have a wonderful day,
John J. Detlefs

coveralls · 2016-07-16T19:34:45Z

Coverage increased (+0.07%) to 81.086% when pulling cc384f1 on jdetle:diffusion_rebase into e801d35 on MDAnalysis:develop.

jdetle · 2016-07-18T19:43:37Z

@richardjgowers Could you give this a once over? I don't want to bug max while he's on his honeymoon but I think everything is ready for a merge.

richardjgowers · 2016-07-18T21:56:54Z

Yep sorry, didn't see you'd finished the new tests.

richardjgowers · 2016-07-18T21:58:30Z

package/MDAnalysis/analysis/diffusionmap.py

+First load all modules and test data ::
+
+   >>> import MDAnalysis as mda
+   >>> import numpy as np


I don't think we actually use np in the code below

richardjgowers · 2016-07-18T22:11:47Z

Ok very minor stuff, otherwise looking good

jdetle · 2016-07-18T22:16:11Z

Thanks, I'm wiped after Scipy but I'll make these changes first thing
tomorrow

On Mon, Jul 18, 2016 at 3:11 PM Richard Gowers [email protected]
wrote:

Ok very minor stuff, otherwise looking good

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#894 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKcARvsPjZuqyr4vy1Ejboonza3autWJks5qW_olgaJpZM4JHd_i
.

Have a wonderful day,
John J. Detlefs

jdetle · 2016-07-19T18:06:50Z

coveralls · 2016-07-19T18:37:38Z

Coverage increased (+0.07%) to 81.103% when pulling 313a174 on jdetle:diffusion_rebase into 054a9de on MDAnalysis:develop.

coveralls · 2016-07-19T19:01:44Z

Coverage increased (+0.07%) to 81.103% when pulling 313a174 on jdetle:diffusion_rebase into 054a9de on MDAnalysis:develop.

kain88-de · 2016-07-20T13:05:49Z

@jdetle Thanks looks great.

orbeckst · 2016-07-20T16:39:16Z

Congratulations @jdetle, major milestone!

Oliver Beckstein
email: [email protected]

Am Jul 20, 2016 um 6:05 schrieb Max Linke [email protected]:

@jdetle Thanks looks great.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

jdetle · 2016-07-20T16:42:47Z

Thanks! Onwards and upwards!

- see his contributions to #894 - also sent invitation to join MDAnalysis as a collaborator

- see his contributions to MDAnalysis#894 - also sent invitation to join MDAnalysis as a collaborator

Diffusion Maps rebase, new PR for final review

@euhruska

q/gHXwgU43h2+Dkye4y3pjLKZ7yN/a/Ffz08WfApOvwen9rzxodFvjawaPd6aPxN D3F2e9FvfM7ETEeNva6deCh1C7vsE1mQu38cONkD1bNITQZKQAqEo960aHFtPUAq f+P61EuVezLcrsTdrNKTiJhs3hXi4S5QBOQJBQH8I8+mvYpotT2/0yv+oapRMsoy dVhB/MWixyA8zAyig5pHV4zKrrok/qqPEZu0DTJoVblqHMNbkc/Z6AkV57w9ade4 +BmhXwXyYYC4JQiHGp9IRdtJgKAoTDnN6XSgamxo3+pcGDZ0k0G1F0AuTgJwbvqK DXrOIq6G1GlUIUwnkqkfkW8GA6z1FuODj/95RxG5YYPrAjP8a9xJ5O8VKoG6Uoqe UV4ER73kv8Ujwagdne4LZYif7p1GXRKzOEIVvJlPypQp2vJvjQ2Lbc84WdHquw7Z yAxrlrme8tW5upnu9tPpPvLssgMFRBMyhFSKTwGBwxMICKbjRQ5bb4KbRbSo/9Hz AhxdK7rDRC2GF3fuPWZ0Cj8bjp6ZllZQjH89smj1mQ4FS76YrYVGbZfu5/89jYTS RWhUi+nWSVar6zcsnXGGFvl6eu+HQkOyeciCXQH+cfIWDFYkiuQdzrsjHVAIFUoL B0sWtG0q5EfdkuFo9ISM =m5vj -----END PGP SIGNATURE----- added @euhruska as author - see his contributions to MDAnalysis#894 - also sent invitation to join MDAnalysis as a collaborator

Eugen Hruska and others added 19 commits July 7, 2016 13:19

new analysis feature: diffusion map

ffcd3f2

description of diffusion map

5fdd7b6

diffusionmap minor fix

ce7dd68

test for diffusionmap

6f8e8cc

diffusionmap faster test

b6f8632

included notes

e773500

initial work for refactor

f2ae4a4

refactor of @euhruska diffusion map

a142422

added tests, fixed logger issue

38f2352

style fixes, protection fixes, added docs fix of some broken areas fixed selections and updated tests accordingly switched order of imports [skip ci]

Now witness the firepower of this fully armed and

32a9655

operational diffusion map

Fixed bug in _single_frame()

aea25a7

Eliminated spectral gap function... for now Fixed matrix multiplication bug Fixed save issue Better single_frame loop Name changes, styling updates and doc fixes.

Refactor of code to stop DiffusionMap inheriting from AnalysisBase

a9a5dbe

Changed code further to make transform a simple iteration. Provided functionality for doing a diffusion map with just a distance matrix.

Removed anisotropic kernel creation. Figured out that I was

9418cdc

unneccesarily exponentiating a diagonalizable matrix. Removed timescaling init, removed tests for anisotropic kernel and timescaling.

Updated docs to remove references to anisotropy, update tutorial [ski…

aa98fa7

…p ci]

Added link to jupyter notebook [skip ci]

b24364e

Added space to get sphinx pages to build properly. [skip ci]

a4c185a

kain88-de reviewed Jul 7, 2016
View reviewed changes

jdetle force-pushed the diffusion_rebase branch from a92788e to b299d89 Compare July 11, 2016 02:05

Updated tests, updated docs

cc384f1

richardjgowers reviewed Jul 18, 2016
View reviewed changes

Fixed docs, returned self

313a174

jdetle force-pushed the diffusion_rebase branch from bf570e7 to 313a174 Compare July 19, 2016 18:37

richardjgowers merged commit 6c6510c into MDAnalysis:develop Jul 20, 2016

orbeckst added a commit that referenced this pull request Jul 22, 2016

added @euhruska as author

d00244c

- see his contributions to #894 - also sent invitation to join MDAnalysis as a collaborator

jdetle pushed a commit to jdetle/mdanalysis that referenced this pull request Aug 14, 2016

added @euhruska as author

e8c8085

- see his contributions to MDAnalysis#894 - also sent invitation to join MDAnalysis as a collaborator

abiedermann pushed a commit to abiedermann/mdanalysis that referenced this pull request Oct 26, 2016

Merge pull request MDAnalysis#894 from jdetle/diffusion_rebase

fe2f620

Diffusion Maps rebase, new PR for final review

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Diffusion Maps rebase, new PR for final review #894

Diffusion Maps rebase, new PR for final review #894

jdetle commented Jul 7, 2016 •

edited

Loading

jdetle commented Jul 7, 2016 •

edited

Loading

kain88-de Jul 7, 2016

coveralls commented Jul 11, 2016 •

edited

Loading

jdetle commented Jul 11, 2016

kain88-de commented Jul 11, 2016

jdetle commented Jul 11, 2016

coveralls commented Jul 16, 2016 •

edited

Loading

jdetle commented Jul 18, 2016

richardjgowers commented Jul 18, 2016

richardjgowers Jul 18, 2016

richardjgowers commented Jul 18, 2016

jdetle commented Jul 18, 2016

jdetle commented Jul 19, 2016

coveralls commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

kain88-de commented Jul 20, 2016

orbeckst commented Jul 20, 2016

jdetle commented Jul 20, 2016

Diffusion Maps rebase, new PR for final review #894

Diffusion Maps rebase, new PR for final review #894

Conversation

jdetle commented Jul 7, 2016 • edited Loading

PR Checklist

jdetle commented Jul 7, 2016 • edited Loading

kain88-de Jul 7, 2016

Choose a reason for hiding this comment

coveralls commented Jul 11, 2016 • edited Loading

jdetle commented Jul 11, 2016

kain88-de commented Jul 11, 2016

jdetle commented Jul 11, 2016

coveralls commented Jul 16, 2016 • edited Loading

jdetle commented Jul 18, 2016

richardjgowers commented Jul 18, 2016

richardjgowers Jul 18, 2016

Choose a reason for hiding this comment

richardjgowers commented Jul 18, 2016

jdetle commented Jul 18, 2016

jdetle commented Jul 19, 2016

coveralls commented Jul 19, 2016 • edited Loading

coveralls commented Jul 19, 2016 • edited Loading

kain88-de commented Jul 20, 2016

orbeckst commented Jul 20, 2016

jdetle commented Jul 20, 2016

jdetle commented Jul 7, 2016 •

edited

Loading

jdetle commented Jul 7, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 16, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading