Skip to content

Ekim/documentation suggestions#202

Merged
maxhutch merged 14 commits intomasterfrom
ekim/documentation-suggestions
Mar 6, 2020
Merged

Ekim/documentation suggestions#202
maxhutch merged 14 commits intomasterfrom
ekim/documentation-suggestions

Conversation

@eddotman
Copy link
Copy Markdown

@eddotman eddotman commented Feb 7, 2020

Citrine Python PR

Description

Some quick suggested changes to the documentation:

  • Typo (?) in an example where a MeanColumn should be a StdColumn -- unless I misunderstood the example.
  • Change "outer product" to "Cartesian product" in explanation of Product design space, since I think this is the correct mathematical concept that describes what is happening.
  • Changed NDE to be NDME consistently
  • Changed description of high NDME to indicate high error and not necessarily high uncertainty.
  • Spell out more detail on the computation of support-weighted F1

PR Type:

  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Adherence to team decisions

  • I have added tests for 100% coverage
  • I have written Numpy-style docstrings for every method and class.
  • I have communicated the downstream consequences of the PR to others.
    ^^^N/A to these -- only changed documentation text.

Edward Kim added 4 commits February 7, 2020 08:39
(outer product implies the elements should have units temp*time -- but this is more like a set of coordinates, which is a cartesian product)
Comment thread docs/source/workflows/design_spaces.rst
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Co-Authored-By: bfolie <bfolie@citrine.io>
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Comment thread docs/source/workflows/performance_workflows.rst Outdated
Edward Kim and others added 7 commits February 11, 2020 08:02
Co-Authored-By: bfolie <bfolie@citrine.io>
Co-Authored-By: gregor-robinson <53885379+gregor-robinson@users.noreply.github.com>
Co-Authored-By: gregor-robinson <53885379+gregor-robinson@users.noreply.github.com>
Co-Authored-By: gregor-robinson <53885379+gregor-robinson@users.noreply.github.com>
Co-Authored-By: gregor-robinson <53885379+gregor-robinson@users.noreply.github.com>
Co-Authored-By: gregor-robinson <53885379+gregor-robinson@users.noreply.github.com>
@eddotman
Copy link
Copy Markdown
Author

I think all suggestions have been added in now -- lmk if I missed anything!

@mcgalcode mcgalcode changed the base branch from develop to master March 4, 2020 00:34
@mcgalcode
Copy link
Copy Markdown
Contributor

cc: @eddotman changed base branch to master as part of the master flow update.

An acceptable NDME depends on how the model is used.
Generally, NDME > 0.9 indicates a model with very high error.
If 0.9 > NDME > 0.6, this model is typically a good candidate for a design workflow.
Lower values of NDME indicate increasingly accurate models.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here would be a nice place to explain what a lower value means as a candidate for a design workflow, continuing the thought from the previous sentence.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed! lmk what you think in the new update

@eddotman eddotman requested a review from gregor-robinson March 5, 2020 10:06
@maxhutch maxhutch merged commit 897afbb into master Mar 6, 2020
@maxhutch maxhutch deleted the ekim/documentation-suggestions branch March 6, 2020 13:59
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.

6 participants