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

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

Closed
seabbs opened this issue Feb 28, 2024 · 23 comments
Closed

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

seabbs opened this issue Feb 28, 2024 · 23 comments
Assignees
Labels

Comments

@seabbs
Copy link
Collaborator

seabbs commented Feb 28, 2024

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

  • Is this actually useful?
  • is there an already in place way to do this
  • conditioned on above is there a better way to do this
  • conditioned on above what else would this need
@seabbs seabbs self-assigned this Feb 28, 2024
@seabbs
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

@SamuelBrand1 thoughts?

@SamuelBrand1
Copy link
Collaborator

SamuelBrand1 commented Feb 29, 2024

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
Copy link
Collaborator

SamuelBrand1 commented Feb 29, 2024

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
Copy link
Collaborator

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

@seabbs
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

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
Copy link
Collaborator

SamuelBrand1 commented Feb 29, 2024

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
Copy link
Collaborator

SamuelBrand1 commented Feb 29, 2024

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
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

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

@seabbs
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

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

@seabbs
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

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
Copy link
Collaborator Author

seabbs commented Feb 29, 2024

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 seabbs added this to the EpiAware 0.1.0 milestone Feb 29, 2024
@seabbs
Copy link
Collaborator Author

seabbs commented Mar 5, 2024

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

@SamuelBrand1
Copy link
Collaborator

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
Copy link
Collaborator Author

seabbs commented Mar 5, 2024

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

@SamuelBrand1
Copy link
Collaborator

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

@seabbs
Copy link
Collaborator Author

seabbs commented Jun 11, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@seabbs
Copy link
Collaborator Author

seabbs commented Jun 28, 2024

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

@seabbs
Copy link
Collaborator Author

seabbs commented Jul 17, 2024

Closing in favour of #276

@seabbs seabbs closed this as completed Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants