Skip to content

Cf adds#309

Open
NikSchlage wants to merge 7 commits intomasterfrom
cf-adds
Open

Cf adds#309
NikSchlage wants to merge 7 commits intomasterfrom
cf-adds

Conversation

@NikSchlage
Copy link
Contributor

Add functions to

  • exponentiate cf objects
  • assign numeric values to entries of cf objects
  • compute the FHT ratio

@NikSchlage NikSchlage requested a review from kostrzewa June 21, 2022 17:29
#' treated as seperate and indepenent, such that the real part of cf is
#' divided by itself and multiplied with n and similarly for the imaginary part.
#'
#' Note that this is generally only allowed on bootstrap samples and mean values,
Copy link
Member

Choose a reason for hiding this comment

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

i don't understand the documentation here... what is this supposed to do?

Copy link
Contributor Author

@NikSchlage NikSchlage Sep 14, 2022

Choose a reason for hiding this comment

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

In the FHT ratio function fht_ratio.cf() I calculate the prefactor z/sqrt(z^2 - 1), i.e. "pre <- z / pow.cf(z * z - num.cf(z), n=0.5)". In this, z is a cf object and consequently z * z is also a cf object, because the objects are multiplied element by element. This means that you cannot simply subtract a numeric value. Therefore, I wrote the function num.cf(), which assigns a numeric value to each entry of the passed cf object, in this case 1, since "num.cf <- function(cf, n=1.)". The result is a cf object with the same dimension as z * z that can be subtracted. So here the value 1 is always subtracted. I achieve the assignment of a numeric value n to all entries via cf$cf <- n * (cf$cf/cf$cf) or cf$icf <- n * (cf$icf/cf$icf). This is what I meant with "the real part of cf is divided by itself and multiplied with n and similarly for the imaginary part.".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I have rewritten my documentation again and hope that it is now more understandable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove the check "if( has_icf(cf) )" also in the functions num.cf()? Just like in pow.cf() don't see that it is needed.

pow.cf <- function(cf, n=1.) {
stopifnot(inherits(cf, 'cf_meta'))
if(inherits(cf, 'cf_orig')) {
cf$cf <- (cf$cf)^n
Copy link
Member

Choose a reason for hiding this comment

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

do you actually want to take a power of the original data or is this an operation which should only be carried out on the mean value and bootstrap samples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to take a power of the original data to keep the function as general as possible. However, in my analysis I passed bootstrapped correlation functions to this function.

R/cf.R Outdated
#'
#' @param cf2pt `cf_orig` object.
#' @param cf3pt `cf_orig` object.
#' @param tau Numeric.
Copy link
Member

Choose a reason for hiding this comment

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

explain what this is please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I have added the explanations.

R/cf.R Outdated
#' FHT ratio
#'
#' @description
#' Computes Feynman-Hellmann theorem (FHT) ratio for given
Copy link
Member

Choose a reason for hiding this comment

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

a formula might help

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is done.

@urbach
Copy link
Member

urbach commented Sep 9, 2022

@NikSchlage this requires additional work!

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.

3 participants