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 user option for similarity measures #40

Merged
merged 28 commits into from
Jun 1, 2023
Merged

Conversation

erikscheurer
Copy link
Contributor

Closes #24

This PR adds options for different similarity measures. Currently L1 and L2 are supported.

@erikscheurer
Copy link
Contributor Author

When implementing a relative L1 norm for example, by which quantity do we divide? For example, we have the two simulation with states 1 and 2, then data=[1,2]. (So still a single scalar value to compare simulations)
If we then calculate the distances between the two we would have diff =[[0 , 1-2],[2-1, 0]] if we do not normalize. How would this look normalized? Normalization by the maximum of the two? If we do a pointwise division with a vector, we would not get a symmetric matrix which does not make much sense right?

@erikscheurer
Copy link
Contributor Author

I see two options now:

  1. Divide by the mean, this would mean, that different units for example would be cancelled out while preserving the symmetry in the distances. In our example this would be diff=[[0 (1-2)/1.5], [(2-1)/1.5, 0]]
  2. Divide by the metric of the "fist" simulation. Divide the matrix row-wise with the vector. diff=[[0, (1-2)/1], [(1-2)/2, 0]]. This would make sense to do because the check if the simulation should be turned active or inactive is done using the row index. To check if the simulation number 0 with data 1 should be turned off, we check if diff[0,:]<tol, therefore we should divide row 0 by the data of simulation 0

@IshaanDesai
Copy link
Member

IshaanDesai commented May 16, 2023

Lets go for the second option, so dividing my the data value of one of the simulations. Which one to pick is our choice, we should only be consistent. As for vectorial quantities, lets say we have a quantity $v$ which we want to use for the comparison. For two micro simulations 1 and 2, this would mean that we have two vectors to compare, $v^1_x, v^1_y, v^1_z$ and $v^2_x, v^2_y, v^2_z$. Let us do a component-wise relative computation then, so we do $\frac{abs(v^1_x - v^2_x)}{v^1_x}$ for all components, and then sum up the relative errors.

@erikscheurer erikscheurer marked this pull request as ready for review May 19, 2023 14:01
@erikscheurer
Copy link
Contributor Author

This should work now as we discussed, do we want unit tests for this?

@erikscheurer erikscheurer requested a review from IshaanDesai May 19, 2023 14:04
Copy link
Member

@IshaanDesai IshaanDesai 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 this! These additions will expand the adaptivity functionality significantly. Minor changes from my side.

@erikscheurer
Copy link
Contributor Author

In the future we may want to switch to a different method of selecting functions. As suggested in #24, the user could provide their own similarity functions which cannot easily be realized with the string-selection.

@erikscheurer erikscheurer requested a review from IshaanDesai May 22, 2023 12:40
@erikscheurer
Copy link
Contributor Author

Do we want to merge this or wait for the documentation and the unit tests? Some documentation would be necessary for the similarity functions.

@IshaanDesai
Copy link
Member

Do we want to merge this or wait for the documentation and the unit tests? Some documentation would be necessary for the similarity functions.

Let me finish the documentation PR, and then we can add documentation here, and merge it.

Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

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

Looks good 👍 only a few documentation related comments from my side. After adding the documentation we can merge.

@erikscheurer erikscheurer requested a review from IshaanDesai June 1, 2023 07:42
@erikscheurer erikscheurer requested a review from IshaanDesai June 1, 2023 09:32
@IshaanDesai IshaanDesai merged commit 295d0f9 into develop Jun 1, 2023
@IshaanDesai IshaanDesai deleted the similarity-functions branch June 1, 2023 11:43
@IshaanDesai IshaanDesai mentioned this pull request Aug 26, 2023
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.

Allow user to provide comparison function for similarity calculation in adaptivity
2 participants