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

Feature/mds example #3728

Closed

Conversation

eleninisioti
Copy link

I have replaced the "converter_multidimensionalscaling_modular.py" example with a meta example and a cookbook page. I provide the .sg and .rst files.

Attention: I am not sure about line 30 of the .sg file. First of all, I am not sure if it should be included and even if it should it does not compile, as make complains about the \ and > symbols.

I've also commented here as I'm not sure where the cookbook should be placed, but I think this is a better place to discuss about it.

@vigsterkr
Copy link
Member

@ElenaNST ok those errors on travis are actually real errors with this patch.

Copy link
Member

@karlnapf karlnapf left a comment

Choose a reason for hiding this comment

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

Great addition, thanks for that.

I commented on some things that need to be addressed. Also, can you please split/squash this into 2 commits: One for adding the example/cookbook/data, and one for the new section in the TOC?

@@ -105,3 +105,13 @@ Neural Networks
:caption: Neural Netwroks

examples/neural_nets/**

Feature Transformations
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be "Feature converters"

Copy link
Author

Choose a reason for hiding this comment

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

Glad to change it, but are you sure about it? I haven't come across the term "Feature converters" in the machine learning literature.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we call these things "converter" internally.
Maybe just leave it as is now, easy to change later

@@ -153,3 +153,15 @@ @article{Quinonero-Candela2005
year = {2005},
pages = {1939--1959},
}
@TECHREPORT{Wickelmaier03anintroduction,
author = {Florian Wickelmaier},
Copy link
Member

Choose a reason for hiding this comment

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

F. Wickelmaier

@@ -153,3 +153,15 @@ @article{Quinonero-Candela2005
year = {2005},
pages = {1939--1959},
}
@TECHREPORT{Wickelmaier03anintroduction,
author = {Florian Wickelmaier},
title = {An introduction to MDS},
Copy link
Member

Choose a reason for hiding this comment

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

We need a better reference here. Maybe David Barbers book?

Copy link
Author

Choose a reason for hiding this comment

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

If you are referring to Bayesian Reasoning and Machine learning, there's no reference to MDS. Maybe Modern Multidimensional Scaling from Borg is ok? (also cited in the documentation of CMultidimensionalScaling)

Copy link
Member

Choose a reason for hiding this comment

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

Yep thats ok

}
@inproceedings{Silva2004SparseMS,
title={Sparse multidimensional scaling using landmark points},
author={Vin de Silva and Joshua B. Tenenbaum},
Copy link
Member

Choose a reason for hiding this comment

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

Abbreviate first names pls

@inproceedings{Silva2004SparseMS,
title={Sparse multidimensional scaling using landmark points},
author={Vin de Silva and Joshua B. Tenenbaum},
booktitle={},
Copy link
Member

Choose a reason for hiding this comment

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

watch you bibtex style, this needs a conference name (check the others to see how we name them)

.. math::
Strain_D(x_1, x_2, \cdots, x_N ) = \frac{\sum_{ij} (b_{ij} -d_{ij})^2}{\sum_{ij} b_{ij}^2}

where :math:`b_{ij}` are the terms of matrix :math:`B`, derived from matrix :math:`D` by applying double centering. This problem is tractable and solved using eigenvalue decomposition from :math:`B=X X'`
Copy link
Member

Choose a reason for hiding this comment

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

Transpose is ^\top in latex.
Please double check the math and make sure every quantity is defined, and used somewhere.


where :math:`b_{ij}` are the terms of matrix :math:`B`, derived from matrix :math:`D` by applying double centering. This problem is tractable and solved using eigenvalue decomposition from :math:`B=X X'`

:cite:`Silva2004SparseMS` introduced a two-step, computationally efficient approximation of classical MDS for use when the number of data points is too large. First, it uses a subset of the initial points as landmarks to apply classical MDS and then, it uses a distance-based triangulation procedure to determine the positions of the remaining points.
Copy link
Member

Choose a reason for hiding this comment

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

Is this demonstrated in here? If so, say so, if not it can be removed

Copy link
Author

Choose a reason for hiding this comment

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

It is not demonstrated, but this converter.set_landmark(False) deactivates it. So, if I remove this paragraph I should write something like "We deactivate an optimization technique". I'm afraid that's a bit vague and I didn't find any relevant case in the cookbooks, so what's your approach? How about just dropping the second sentence?

Copy link
Member

Choose a reason for hiding this comment

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

I would explicitly demostrate this in a second step in the example. First without and then with, mentioning it


.. sgexample:: multidimensional_scaling:create_features

We calculate the Euclidean distance between our features.
Copy link
Member

Choose a reason for hiding this comment

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

use sgclass for Euclidean distance here


.. sgexample:: multidimensional_scaling:calculate_distance_matrices


Copy link
Member

Choose a reason for hiding this comment

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

a lot of whitespace here

#![calculate_distances_after]

#![calculate_distance_matrices]
RealMatrix distance_matrix_after = distance_after.get_distance_matrix()
Copy link
Member

Choose a reason for hiding this comment

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

move these to calculate_distances_before and calculate_distances_after

No need for its own section

@stale
Copy link

stale bot commented Feb 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 26, 2020
@stale
Copy link

stale bot commented Mar 4, 2020

This issue is now being closed due to a lack of activity. Feel free to reopen it.

@stale stale bot closed this Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants