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

Add description of inverse transform in ICAConverter cookbook #4390

Merged
merged 5 commits into from Aug 14, 2018

Conversation

@vinx13
Copy link
Member

@vinx13 vinx13 commented Aug 2, 2018

No description provided.

@@ -19,3 +19,7 @@ Features converted = ica.transform(feats)
RealMatrix mixing_matrix = ica.get_real_matrix("mixing_matrix")
RealMatrix unmixed_signal = converted.get_real_matrix("feature_matrix")
#![extract]

#![inverse_transform]
Features feats_original = ica.inverse_transform(converted)

This comment has been minimized.

@karlnapf

karlnapf Aug 3, 2018
Member

pls also extract the matrix here (even if it is not shown in the cookbook) for the sake of integration testing ...

@@ -29,6 +29,9 @@ We can also extract the estimated mixing matrix.

.. sgexample:: independent_component_analysis_fast.sg:extract

:sgclass:`CICAConverter` supports inverse transformation. We can mix signals using the mixing matrix learnt before.

This comment has been minimized.

@karlnapf

karlnapf Aug 3, 2018
Member

minor: newline after period

Copy link
Member

@karlnapf karlnapf left a comment

Thats a nice addition! thx

@karlnapf
Copy link
Member

@karlnapf karlnapf commented Aug 3, 2018

the data update is fishy

Features feats_original = ica.inverse_transform(converted)
#![inverse_transform]

RealMatrix signal_original = feats_original.get_real_matrix("feature_matrix")

This comment has been minimized.

@karlnapf
Copy link
Member

@karlnapf karlnapf commented Aug 3, 2018

something is still weird with the data

@vinx13
Copy link
Member Author

@vinx13 vinx13 commented Aug 3, 2018

@karlnapf i guess the data change of the old line now is because of float point precision error, they are very near

@karlnapf
Copy link
Member

@karlnapf karlnapf commented Aug 3, 2018

No I mean all those removed files

@vinx13 vinx13 force-pushed the vinx13:cookbook/inv_transform branch from 702f7e6 to 5582ce5 Aug 3, 2018
@vinx13
Copy link
Member Author

@vinx13 vinx13 commented Aug 3, 2018

i rebased my data pr

@karlnapf
Copy link
Member

@karlnapf karlnapf commented Aug 8, 2018

could you try again?

@vinx13 vinx13 force-pushed the vinx13:cookbook/inv_transform branch from 5b7b738 to 681e5b5 Aug 9, 2018
@vinx13
Copy link
Member Author

@vinx13 vinx13 commented Aug 13, 2018

@karlnapf tests passed

@vigsterkr vigsterkr merged commit c89317e into shogun-toolbox:develop Aug 14, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants