CDCgov / Rt-without-renewal

https://cdcgov.github.io/Rt-without-renewal/
Apache License 2.0
12 stars 2 forks source link

Functions that allow easy creation of new but related structs #90

Open seabbs opened 4 months ago

seabbs commented 4 months ago

For many of our use cases it would be great to be able to create new structs for some/all of our structs with just a single input change.

It looks like the suggested approach to do this is to create a function that dispatches on the struct and creates a new struct.

I've created a test of this in issue90

Questions

seabbs commented 4 months ago

@SamuelBrand1 thoughts?

SamuelBrand1 commented 4 months ago

This is handy functionality.

Maybe, have transform as transfrom(ep::EpiData; kwargs...) and use arg matching on method to do either of the two main constructor functions for EpiData?

Atm, its easy to swap in a new discrete GI; but that would let us swap in a new continuous GI as well.

SamuelBrand1 commented 4 months ago

Basically, I'm thinking remake described here. Which is used extensively to transform DEProblem objects in optimization/Bayes by modifying their initial condition and parameters (and anything else).

The methods for remake are doc here.

And the src code is quite readable, we wouldn't need all the helper functions but I like that pattern.

SamuelBrand1 commented 4 months ago

In fact, can I request that we call it remake and it works on any of the structs we use?

seabbs commented 4 months ago

So yes the idea was to make one for all the structs and the remake pattern is just what I was looking for.

We need to define a method for every struct we use right?

SamuelBrand1 commented 4 months ago

So yes the idea was to make one for all the structs and the remake pattern is just what I was looking for.

We need to define a method for every struct we use right?

That might be easiest, but in the remake src they seem to have a helper function struct_as_named_tuple, which might let you just hit the existing constructor functions

as an example maybe aim for this pseudocode

function remake(my_struct::AbstractModel; kwargs_to_change...)

ty = typeof(my_struct)
ty_all_kw_args = fieldnames(ty)
nm_tuple_not_changing = get_values_for_kw_not_changing(my_struct, ty_all_kw_args, kwargs_to_change)

all_kws = merge(kwargs_to_change, nm_tuple_not_changing)
#Use constructor
ty(all_kws)

end
SamuelBrand1 commented 4 months ago

struct_as_named_tuple Nice macro!

Actually, we could just lift this with reference to SciML. We'd just need this and their most general remake method (remake(thing; kwargs...)).

Its an elegant way of doing the

nm_tuple_not_changing = get_values_for_kw_not_changing(my_struct, ty_all_kw_args, kwargs_to_change)

pseduocode above.

seabbs commented 4 months ago

Here should we be lifting or should we be depending on them (just for these things?)

seabbs commented 4 months ago

More generally we don't have any namespace management of the functions we use (just the packages) like we would in R. Is that because we don't need to (and so this represents best practice) or is this really something we should have but don't?

SamuelBrand1 commented 4 months ago

Here should we be lifting or should we be depending on them (just for these things?)

I think SciMLBase is a heavy dep for a couple of utility functions? Maybe if they ever spin out SciMlUtils?

SamuelBrand1 commented 4 months ago

More generally we don't have any namespace management of the functions we use (just the packages) like we would in R. Is that because we don't need to (and so this represents best practice) or is this really something we should have but don't?

In all the Julia I've seen namespace management is just up to modules and package exports and python-like patterns like

import EpiAware as epa
epa.my_epi_aware_func(args...)

I imagine its heading towards things like this, because it has the same potential namespace issues as python can have?

seabbs commented 4 months ago

ah okay so its not that it just magically works but rather that its a rough edge

seabbs commented 4 months ago

I think SciMLBase is a heavy dep for a couple of utility functions?

Yeah I was really asking if there was a clever way to just dep on the parts of their package that you wanted but it sounds like no.

seabbs commented 4 months ago

A final org queston. When we define function generics (like remake and generate_latent) is the best practice to define the generic (meaning the method that dispatches if a specific version isn't defined) all in one place (i.e all generics together) or to try and define all uses of a function together or to something else?

Looking at docs practice its definitely best practice to document functions all together

seabbs commented 4 months ago

The current plan is to import sciml base for this functionality and see how that works

SamuelBrand1 commented 4 months ago

I think SciMLBase is a heavy dep for a couple of utility functions?

Yeah I was really asking if there was a clever way to just dep on the parts of their package that you wanted but it sounds like no.

Internally, you can use e.g.

using SciMLBase: remake

But I'm not sure how much that lightens the dep vs namespace management (which is also useful).

seabbs commented 4 months ago

I think that will just help with namespace management but is worth doing anyway for clarity I think

SamuelBrand1 commented 4 months ago

Personally, I've come round to just use it.

seabbs commented 3 weeks ago

This functionality also seems to be supported by Accessors.jl in a nice general way. Perhaps we could link out to that (it almost feels like we need a hints and tips doc section or something)?

SamuelBrand1 commented 3 weeks ago

Yes I think having Accessors just does what we want, nice find. I see that Turing uses this internally but hadn't clocked it was so useful.

SamuelBrand1 commented 1 week ago

@seabbs I think we've gone for Accessors.jl? Close this?

seabbs commented 1 week ago

I think we need to document that option somewhere clear and then can close (perhaps the proposed FAQ)