xKDR / Survey.jl

Analysis of complex surveys
https://xkdr.github.io/Survey.jl/
GNU General Public License v3.0
50 stars 19 forks source link

`bydomain` is hardcoded for `bootstrap` variance #286

Open smishr opened 1 year ago

smishr commented 1 year ago

See line 22-27 in by.jl

    for i = 1:nd
        filtered_dx = filter(!isnan, Xt_mat[i, :] .- X.statistic[i])
        push!(ses, sqrt(sum(filtered_dx .^ 2) / length(filtered_dx)))
    end
    replace!(ses, NaN => 0)
    X.SE = ses

This is hardcoding the variance for "bootstrap" type ReplicateDesign. For future additions, like JKn, the variance is calculated differently.

TODO: Update bydomain function with modularity for current and future methods of variance estimation.

ayushpatnaikgit commented 1 year ago

We need to update all the functions and think carefully. How about:

abstract type ReplicateDesign end 

struct BootstrapDesign <: ReplicateDesign
       end

struct JackknifeDesign <: ReplicateDesign
       end

?

Then we can do multiple dispatch instead of if else ladders.

I wish there was inheritance. We could have done BoostrapDesign <: ReplicateDesign <: SurveyDesign

smishr commented 1 year ago

We need to update all the functions and think carefully. How about:

abstract type ReplicateDesign end 

struct BootstrapDesign <: ReplicateDesign
       end

struct JackknifeDesign <: ReplicateDesign
       end

?

Then we can do multiple dispatch instead of if else ladders.

I wish there was inheritance. We could have done BoostrapDesign <: ReplicateDesign <: SurveyDesign

Yes something like this should improve the modularity of the code.

But we need to throughly think this through. All other parts of the package should be made modular as well at the same time

How do other packages solve the inheritance issues?

asinghvi17 commented 1 year ago

I haven't looked through the code too much, but a partial solution presents itself:

abstract type AbstractSurveyDesign end

struct SurveyDesign <: AbstractSurveyDesign
...
end

abstract type AbstractReplicateDesign <: AbstractSurveyDesign end

struct BootstrapDesign <: AbstractReplicateDesign
       end

struct JackknifeDesign <: AbstractReplicateDesign
       end

so you basically introduce a top-level abstract type, which SurveyDesign and AbstractReplicateDesign both subtype. Then, generic methods can be dispatched on AbstractSurveyDesign. I've used this pattern, and seen it used, in a number of places (most notably as Number -> (Real -> ..., Complex)).

smishr commented 1 year ago

I haven't looked through the code too much, but a partial solution presents itself:

abstract type AbstractSurveyDesign end

struct SurveyDesign <: AbstractSurveyDesign
...
end

abstract type AbstractReplicateDesign <: AbstractSurveyDesign end

struct BootstrapDesign <: AbstractReplicateDesign
       end

struct JackknifeDesign <: AbstractReplicateDesign
       end

so you basically introduce a top-level abstract type, which SurveyDesign and AbstractReplicateDesign both subtype. Then, generic methods can be dispatched on AbstractSurveyDesign. I've used this pattern, and seen it used, in a number of places (most notably as Number -> (Real -> ..., Complex)).

@ayushpatnaikgit @codetalker7 what do you guys think here?

asinghvi17 commented 1 year ago

Ah, I just realized I suggested two different approaches :D - let's discuss on the call if any of those make sense.