Skip to content

Fix issue 43#44

Open
justushelo wants to merge 4 commits intoSimulation-Decomposition:mainfrom
justushelo:fix-issue-43
Open

Fix issue 43#44
justushelo wants to merge 4 commits intoSimulation-Decomposition:mainfrom
justushelo:fix-issue-43

Conversation

@justushelo
Copy link

Fixed sensitivity indices SOE calculation and additionally fix decomposition logic to follow Matlab.

Copy link
Member

@tupui tupui left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @justushelo !

Could you add a/some test which would have been failing without these changes and now pass?

foe : ndarray of shape (n_factors, 1)
First-order effects (also called 'main' or 'individual').
soe : ndarray of shape (n_factors, 1)
soe_full : ndarray of shape (n_factors, 1)
Copy link
Member

Choose a reason for hiding this comment

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

Why renaming?

Copy link
Author

Choose a reason for hiding this comment

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

I thought it could help any future debugging to have the same variable name soe_full as it can be found in the Matlab function.

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 not do it

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.

2 participants