Skip to content

Conversation

@DanTanAtAims
Copy link
Collaborator

Add documentation for the results store.

Reefmod Engine Outputs and scenario information are recorded from the ReefMod Engine API
after each scenario(s) run, not before.

## ResultStore Struct
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
## ResultStore Struct
## Result Store

Struct is an implementation detail that doesn't feel right to have in user-facing documentation.

Also, "Result Store", "results store", "ResultStore", pick one and use it consistently throughout 😸

Copy link
Collaborator

@ConnectedSystems ConnectedSystems left a comment

Choose a reason for hiding this comment

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

Requesting some minor changes for consistency and formatting

dataframe and results YAX dataset. This is because the api forces counterfactuals to be
evaluated with every intervention run.

Model inputs are stored in the`results` field of the store and contains the following
Copy link
Collaborator

Choose a reason for hiding this comment

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

Model inputs are stored in "results"? Is that correct? And if so, isn't that confusing?

Copy link
Collaborator Author

@DanTanAtAims DanTanAtAims Oct 9, 2024

Choose a reason for hiding this comment

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

We can probably more dhw and cyclone category to a different field or rename the field

Copy link
Collaborator

@ConnectedSystems ConnectedSystems left a comment

Choose a reason for hiding this comment

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

Thanks

@ConnectedSystems ConnectedSystems merged commit 40a68d6 into main Oct 9, 2024
1 check passed
@ConnectedSystems ConnectedSystems deleted the update-docs branch October 9, 2024 04:01
@ConnectedSystems ConnectedSystems mentioned this pull request Oct 9, 2024
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.

3 participants