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

Improve documentation on setting extent and center (addresses #961) #962

Merged
merged 2 commits into from Jun 12, 2018

Conversation

Projects
None yet
3 participants
@heplesser
Contributor

heplesser commented Jun 4, 2018

Added paragraphs to sections on setting extent and center use of values that are exact binary numbers to avoid roundoff.

Addresses #961, which cannot be fixed in a perfect and general way: there is simply no general way in which we can avoid rounding errors here (at least not one that would be relatively straightforward to implement), and NEST needs to terminate if rounding errors would need to inconsistent placement of nodes in the Topology-internal data structures.

Added paragraphs to sections on setting extent and center suggesting
use of values that are exact binary numbers to avoid roundoff.
Addresses #961, which cannot be fixed in a perfect and general way.
@jessica-mitchell

Just a question for clarification, do the 'components' refer to the '2-element tuple of floats' mentioned in the preceding paragraph?
I suggest on line 279 that 'Also' be removed as I don't think it adds much to the sentence (if you want to keep it, please add a comma after it).
There is a comma splice in that sentence too; please either put a full stop or semicolon after fractions, and make "see Sec . . ." a complete sentence or put it in parentheses.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Jun 5, 2018

Contributor

@jessica-mitchell Yes, I mean the "2-element tuple of floats". I will make this more consistent. I will also integrate your other suggestions.

Contributor

heplesser commented Jun 5, 2018

@jessica-mitchell Yes, I mean the "2-element tuple of floats". I will make this more consistent. I will also integrate your other suggestions.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Jun 5, 2018

Contributor

@jessica-mitchell I have now updated the text.

Contributor

heplesser commented Jun 5, 2018

@jessica-mitchell I have now updated the text.

@jessica-mitchell

Looks good

@heplesser heplesser merged commit e55b846 into nest:master Jun 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@heplesser heplesser deleted the heplesser:fix-961-extent-values branch Aug 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment