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

diffusion maps cookbook and converter factory #4340

Conversation

shubham808
Copy link
Contributor

No description provided.

Diffusion Maps
==============================

Diffusion Maps is a non-linear dimensionality reduction method that uses
Copy link
Member

Choose a reason for hiding this comment

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

Maybe shorten the sentence slightly.

@@ -0,0 +1,37 @@
==============================
Copy link
Member

Choose a reason for hiding this comment

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

pls adjust to text

@karlnapf
Copy link
Member

Could you add a data file for the new example?
Otherwise good to go!


Diffusion Maps is a non-linear dimensionality reduction method that uses
eigenfunctions of Markov matrices to construct coordinates called diffusion maps
that can be used to generate efficient representations of complex geometric structures.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a key equation that we could show here? You could shop around the web to see what other libraries use to describe this

@shubham808
Copy link
Contributor Author

@karlnapf Travis is failing.... any ideas ? :)

@shubham808 shubham808 force-pushed the feature/converter_factory_and_diffusion_maps_cookbook branch from 1c2960f to 1db5334 Compare June 20, 2018 15:48
@shubham808 shubham808 force-pushed the feature/converter_factory_and_diffusion_maps_cookbook branch from bae2cca to 1e93c7f Compare June 20, 2018 16:10
Example
-------

we create CDenseFeatures (RealFeatures, here 64 bit float values).
Copy link
Member

Choose a reason for hiding this comment

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

Capital W


.. sgexample:: diffusionmaps.sg:create_features

We create the :sgclass:`CDiffusionMaps` instance, and set its parameters. The diffusion kernel :math: `k` must satisfy
Copy link
Member

Choose a reason for hiding this comment

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

We create "a".
Also pls start new sentences in new lines

We create the :sgclass:`CDiffusionMaps` instance, and set its parameters. The diffusion kernel :math: `k` must satisfy
the following properties:

1. :math: `k` is symmetric :math: `{\bf k}(x, y) = {\bf k}(y, x)`
Copy link
Member

Choose a reason for hiding this comment

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

this should be said in the intro


.. sgexample:: diffusionmaps.sg:set_parameters

Then we apply DiffusionMaps, which gives us the distance embeddings.
Copy link
Member

Choose a reason for hiding this comment

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

remove "the"

Also, please spell it "diffusion maps"

@karlnapf
Copy link
Member

LGTM.

@vinx13 did you change any of the API here? Do we need to change anything for that?

@karlnapf
Copy link
Member

No idea why windows fails, restarted.
Travis is OK so all code seems good

@vinx13
Copy link
Member

vinx13 commented Jun 21, 2018

Yes, we have new api in feature/transformers branch. Something like
features = converter.transform(features)
And there is factory method transformer.

@karlnapf
Copy link
Member

Ah ok that is a feature branch then.
So we will merge this, and then refactor upon feature branch merging

@shubham808 shubham808 force-pushed the feature/converter_factory_and_diffusion_maps_cookbook branch 2 times, most recently from f07e583 to 0cae94f Compare June 21, 2018 09:51
#![create_features]

#![set_parameters]
Kernel k = kernel("GaussianKernel", cache_size=10, log_width=10.0)
Copy link
Member

Choose a reason for hiding this comment

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

pls remove the cache size. Sorry didnt spot this earler

@shubham808 shubham808 force-pushed the feature/converter_factory_and_diffusion_maps_cookbook branch from 0cae94f to 3997028 Compare June 21, 2018 11:34
@karlnapf
Copy link
Member

Very useful stuff, thanks! :)

@karlnapf karlnapf merged commit e3d4c02 into shogun-toolbox:develop Jun 22, 2018
ktiefe pushed a commit to ktiefe/shogun that referenced this pull request Jul 30, 2019
* diffusion maps cookbook and converter factory
* data update
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

3 participants