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

Function parameter documentation #304

Merged
merged 1 commit into from
Jul 5, 2023
Merged

Function parameter documentation #304

merged 1 commit into from
Jul 5, 2023

Conversation

odunbar
Copy link
Collaborator

@odunbar odunbar commented Jun 29, 2023

Purpose

Fixes #300
Addresses GRF docs part of #282

NB this is merely documenting what is currently implemented, improvements to the API will come soon.

Content

  • Adds the constraint keyword to fix base.show() bug with function distributions
  • runs show(parameter_distribution) during tests to appease codecov
  • Adds a section and example of creating and plotting GaussianRandomFieldInterface object and samples.
    Relevant docs section found here and here.

  • I have read and checked the items on the review checklist.

@odunbar odunbar force-pushed the orad/function_parameters_2 branch from 0363188 to 3aade9b Compare June 29, 2023 01:26
@odunbar odunbar requested a review from haakon-e June 29, 2023 01:30
@odunbar odunbar force-pushed the orad/function_parameters_2 branch 2 times, most recently from 846b125 to 16bede9 Compare June 29, 2023 20:23
Copy link
Member

@haakon-e haakon-e left a comment

Choose a reason for hiding this comment

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

Good PR! Added a few minor comments, but I basically think this can be merged. We can always make further improvements to the docs in the future as people start using the function parameters!

An aside:
It seems the terminology "class" or "subclass" is used a few locations throughout the documentation to describe parameter distributions both relating to function parameters, but also in general. I think it would be good to be a bit more precise to refer to these as abstract types and structs. That said, I think it's best to discuss and make those changes in a separate PR.

docs/src/parameter_distributions.md Outdated Show resolved Hide resolved
```
We plot 4 samples of this distribution. Samples are taken over the (30-dimensional) degrees of freedom, and then we apply the `transform_unconstrained_to_costrained` map to (i) build the function distribution, (ii) evaluate it on the numerical grid, and (iii) constrain the output with our prescribed bounds.
```@example snip_fun
shape = [length(pp) for pp in points]
Copy link
Member

Choose a reason for hiding this comment

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

could ndims be used here instead to illustrate the usage of that method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the functional parameter ndims only gives the total number of eval points, where as here for plotting we need the breakdown of # eval points per dimension. It might be worth storing this grid-shape internally however, for future use in reshaping. I'll create a separate issue, I presume it is obtainable from the GRF package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have appended this into #282

function distribution docs

run show(parameter_distribution)

format

updated from review
@odunbar odunbar force-pushed the orad/function_parameters_2 branch from 16bede9 to 9fcc796 Compare July 5, 2023 22:00
@odunbar
Copy link
Collaborator Author

odunbar commented Jul 5, 2023

I've also created an issue for the terminology #308

@odunbar
Copy link
Collaborator Author

odunbar commented Jul 5, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 5, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 7b6149c into main Jul 5, 2023
13 checks passed
@bors bors bot deleted the orad/function_parameters_2 branch July 5, 2023 22:29
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.

Base.show(::ParameterDistribution) error with Gaussian Random Fields
2 participants