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

RFC: file structure #44

Closed
st-- opened this issue Oct 29, 2021 · 1 comment · Fixed by #47
Closed

RFC: file structure #44

st-- opened this issue Oct 29, 2021 · 1 comment · Fixed by #47

Comments

@st--
Copy link
Member

st-- commented Oct 29, 2021

Currently, the code is split across four files:

  • ParameterHandling.jl (18 lines): imports, exports, includes
  • flatten.jl (127 lines): the base functionality (discussion whether to move this into a separate package in Extracting flatten/unflatten into lightweight package? #43)
  • test_utils.jl (95 lines): interface tests
  • parameters.jl (247 lines, more than the other three together): everything else
    • AbstractParameter and value(),
    • value() for base types,
    • positive,
    • bounded,
    • fixed,
    • deferred,
    • orthogonal,
    • positive_definite

Following the discussion around positive_definite in #41, it seems that we might turn this into several: lower_triangular (no constraint on diagonal), lower_triangular_positive_diag (positive diagonal), and positive_definite for returning a PDMat.

Before making parameters.jl even longer (I already find it to be at if not above the upper end of how large a file can be to still keep a good overview of disparate content), I was wondering if it might make sense to split up parameters.jl - there's of course plenty of options, here's the ones I can see immediately:

A: one file per "item", i.e. perhaps base.jl for AbstractParameter and value() for base types, then positive.jl, bounded.jl, fixed.jl, etc.

B: grouped into less files, perhaps as follows:

  • base.jl / interface.jl / something like that: AbstractParameter and value() for base types, perhaps also including fixed and deferred, or keeping the latter two separate
  • scalar.jl: positive, bounded
  • matrix.jl: orthogonal, positive_definite &c.

C: status quo.

It would also be an option (composable with any of the above) to move some of the base definitions, e.g. AbstractParameter, function value end, into ParameterHandling.jl.

What would be your preference? Maybe another suggestion?

@willtebbutt
Copy link
Member

It would also be an option (composable with any of the above) to move some of the base definitions, e.g. AbstractParameter, function value end, into ParameterHandling.jl.

I'd rather not do this - I'm generally not a fan of defining stuff in the main source file (I generally try to minimise it).

I think I like something like option B the most. I like the idea of putting the api in one file, implementations for base types in another, and stuff for AbstractParameters in other files.

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 a pull request may close this issue.

2 participants