Add nextfloat(::BigFloat, n), prevfloat(::BigFloat,n) #31310

narendrakpatel · 2019-03-10T11:09:44Z

New constructor for BigFloat. refer this
Improve nextfloat/prevfloat and introduce nextfloat! and prevfloat!
Introduce nextfloat(::BigFloat, n) and prevfloat(::BigFloat, n)
Tests for constructor, nextfloat and prevfloat

StefanKarpinski · 2019-03-11T18:21:24Z

base/mpfr.jl

 end

+nextfloat(x::BigFloat, n::Int) = nextfloat!(BigFloat(x, new=true), n)


Technically when n == 0 we could just return x. Not sure it's worth the complexity though.

Actually, the special case n==0 is already considered in nextfloat!(). Note that nextfloat() had to return a copy of x, so anyhow we would need to call Bigfloat constructor. Thus no special case was specifically created for this function.

Actually, the special case n==0 is already considered in nextfloat!()

But that's too late: BigFloat(x, new=true) has already allocated a new copy of x by the time nextfloat! gets the value.

My argument is that, nextfloat() needs to return a new copy of x, as it does not ends with !. So even in case n==0, we would need to create a copy of x. Thus, no computation is wasted.

My argument is that, nextfloat() needs to return a new copy of x

Returning the same value is not a mutating operation. Since BigFloats are considered to be immutable it's fine to return the same value instead of allocating a new identical value.

Does this look good?

nextfloat(x::BigFloat, n::Int) = n == 0 ? x : nextfloat!(BigFloat(x, new=true), n)

Yes, that's just it.

StefanKarpinski · 2019-03-11T18:23:08Z

This is looking fairly good. There's a key API question in here: do we want BigFloat(x, new=true) to be the official way to ask for a newly allocated big float?

narendrakpatel · 2019-03-11T19:38:36Z

We had a long discussion on what the API of the BigFloat should look like here. I believe the summary of the whole discussion can be found here. @JeffreySarnoff can elaborate more. What are your views on it?

StefanKarpinski · 2019-03-11T19:39:40Z

That's fine, I'm calling broader attention to it to get some agreement before merging.

simonbyrne · 2019-03-11T20:53:18Z

base/mpfr.jl

-        return z
-    end
+function BigFloat(x::BigFloat, r::MPFRRoundingMode=ROUNDING_MODE[];
+                    precision::Integer=MPFR.precision(x), new::Bool=false)


Suggested change

precision::Integer=MPFR.precision(x), new::Bool=false)

precision::Integer=MPFR.precision(x), new::Bool=precision!=MPFR.precision(x))

simonbyrne · 2019-03-11T20:54:02Z

base/mpfr.jl

-    end
+function BigFloat(x::BigFloat, r::MPFRRoundingMode=ROUNDING_MODE[];
+                    precision::Integer=MPFR.precision(x), new::Bool=false)
+    precision == MPFR.precision(x) && !new && return x


Suggested change

precision == MPFR.precision(x) && !new && return x

new || return x

Does this sound good?

precision != MPFR.precision(x) && !new && @warn "new=false is ignored because precision differ" && return BigFloat(x, precision=precision) new || return x

We definitely don't want random warnings in unexpected corner cases. If you called BigFloat(x) and didn't ask for a definitely new value it should be fine to either give you a new BigFloat or to give you the same one since the official API for BigFloats is that they are immutable, so you shouldn't be able to tell the difference anyway.

The only legitimate use case for BigFloat(x, new=true) is when you're copying an input from somewhere and then are going to mutate that input in the internal implementation of an algorithm. That's why I'm not convinced about this API: if you wanted a new copy of an incoming value so that you can mutate it, wouldn't you also want to copy the precision?

The precision is being copied. In the constructor, default value of precision is precision(x).

Ok, then with @simonbyrne's new default value I'm more ok with this.

We should decide about using new as a parameter first. imo my prior note gives a way to determine that which strips away some of the dross.

I think this does what we want and avoids the controversial.

defined in mpfr and not exported, these respect the precision of x

function duplicate(x::BigFloat) z = BigFloat(; precision = precision(x)) ccall((:mpfr_set, :libmpfr), Int32, (Ref{BigFloat}, Ref{BigFloat}, Int32), z, x, 0) return z end function nextfloat!(x::BigFloat) ccall((:mpfr_nextabove, :libmpfr), Int32, (Ref{BigFloat},), x) != 0 return x end function prevfloat!(x::BigFloat) ccall((:mpfr_nextbelow, :libmpfr), Int32, (Ref{BigFloat},), x) != 0 return x end

defined in mpfr.jl and exported, these also respect the precision of x
without influencing any other aspects of the BigFloat interface.

nextfloat(x::BigFloat) = nextfloat!(duplicate(x)) prevfloat(x::BigFloat) = prevfloat!(duplicate(x))

nextfloat(x,n), prevfloat(x,n), nextfloat!(x,n), prevfloat!(x,n) accordingly

^ revised so nextfloat(x) != x

+:100: to the non-controversial approach of introducing an unexported duplicate function for this PR. That let's us get this PR merged adding the desired nextfloat and prevfloat methods and we can have a separate discussion of the best API for creating copies.

@narendrakpatel please ^, do as we discussed last night.
Don't worry about the failing tests now -- a closer look shows your PR does not trigger the fails.

Keno · 2019-03-14T18:31:37Z

We took a brief look at this during triage. Two comments: We don't like the new keyword argument, but that seems to have been discussed extensively already. The other observation is that this seems like it changes the rounding mode behavior of the nextfloat/prevfloat functions and is thus a breaking change.

narendrakpatel · 2019-03-14T18:58:05Z

@Keno If we implement the _duplicate like this then I think that issue will be solved.

function _duplicate(x::BigFloat)
    z = BigFloat(;precision=precision(x))
    ccall((:mpfr_set, :libmpfr), Int32, (Ref{BigFloat}, Ref{BigFloat}, MPFRRoundingMode),
            z, x, ROUNDING_MODE[])
    return z
end

Basically only change in proposed implementation would be the use of z=BigFloat(x, precision=precision(x)) instead of z=BigFloat(x) in nextfloat/prevfloat functions.
Shall I make these changes in the PR?

JeffreySarnoff · 2019-03-14T20:21:40Z

@narendrakpatel Because there is now exported nextfloat(::BigFloat) and prevfloat(::BigFloat) and they use precision(BigFloat) rather than the precision of their argument, to do anything other than that in implementing nextfloat(::BigFloat, ::Int) and prevfloat(::BigFloat, ::Int) would require (for logical consistancy) that the current nextfloat and prevfloat implementations change to use the precision of their arguments.

That change would be "breaking" -- so it is unavailable until Julia v2.
This means that the implementation of nextfloat(::BigFloat, ::Int) and prevfloat(::BigFloat, ::Int) must be limited to the equivalent of repeatedly calling the currently exported versions nextfloat(::BigFloat) and prevfloat(::BigFloat).

All of the good thought and refinement that we have developed over the past days will be useful. Either as contributing to the "reform" of BigFloats for v2, or in providing important principles for a complementary package, say LargeFloats.

I know others are paying attention to what is and is not "breaking". It's most important to the viability of Julia as the user base grows. My perspective is more about where we might be going, or how I might go where I like with Julia.

So .. given this, why not take a shot at cleanly coding the "works like repeated calls to _" versions of these functions for BigFloat.

JeffreySarnoff · 2019-03-14T20:43:11Z

Is that ^ where we are at, @Keno?

Keno · 2019-03-14T20:47:12Z

I'm just the scribe here ;). If nextfloat(::BigFloat, n) didn't work before, it can do whatever, we just can't change any behavior that worked before (if we think it's minor, we can make that decision and put it in a minor version, but that would need to be explicitly decided and called out).

JeffreySarnoff · 2019-03-14T21:09:02Z

As far as I know, nobody uses prevfloat(BigFloat), nextfloat(BigFloat) -- perhaps there is someone who once did something with that -- but the way people have related to BigFloats is not one that naturally leads to thinking of their previous and next floating point values. Perhaps the best evidence of this is that until I noticed it, there were no mentions of the lack of an nth next or nth prev.

There is a good deal of support for the PR as @narendrahkpatel is about to provide.
@narendrakpatel Now Is The Time.

JeffreySarnoff · 2019-03-15T08:04:21Z

Until v1.1, there was no precision keyword to use constructing a BigFloat.
There was an unnamed positional arg which would pass along a value for the precision, which was of heroically little use because as soon as on did anything numerical with a BigFloat, the precision of the result has been slammed with the current value of precision(BigFloat). Other than looking at digits, using the unnamed second arg for setting custom precision has not been generally helpful -- and so, has not been seen as a tool of the BigFloat realm.

That nextfloat and prevfloat stepped bits at the current value of precision(BigFloat) is exactly how they will behave going forward when given a BigFloat that has been constructed using the current precision(BigFloat). Which is exactly as they have behaved when given a BigFloat that has been constructed using the current precision(BigFloat) <or any other precision -- so what? people do not use BigFloats with varying precisions in nextfloat, prevfloat>.

(a) there is no pattern of use for nextfloat, prevfloat with BigFloats because the question of precision has lacked resonance beyond 256 bits, that's good maybe for this 128 bits or for that 1024 bits (b) the sort of numerical processing that benefits from adjusting the precision does not work very well when the number type keeps slamming the current precision onto old and new results alike.

That nextfloat and prevfloat will respect the precision of the values given means they will work just as they have worked (had they been used) as the values given had been of the current precision (had they been given). Allowing more precise and well-mannered numerical exploration going forward is only upside.

narendrakpatel · 2019-03-19T15:56:14Z

@StefanKarpinski @Keno Does this PR require any other changes?

rfourquet · 2019-03-20T16:33:18Z

base/mpfr.jl

-    ccall((:mpfr_nextabove, :libmpfr), Int32, (Ref{BigFloat},), z) != 0
-    return z
+function nextfloat!(x::BigFloat, n::Integer)
+    n==0 && return x


For style consistency, it seems better to add space around == (same for prevfloat!).

But actually, this line seems unnecessary, the same result is achieved by an empty for-loop below.

rfourquet · 2019-03-20T16:39:58Z

base/mpfr.jl

+nextfloat(x::BigFloat, n::Integer) = n==0 ? x : nextfloat!(_duplicate(x), n)
+prevfloat(x::BigFloat, n::Integer) = n==0 ? x : prevfloat!(_duplicate(x), n)
+
+nextfloat!(x::BigFloat) = (ccall((:mpfr_nextabove, :libmpfr), Int32, (Ref{BigFloat},), x); x)


Why not instead add a default argument, like nextfloat!(x::BigFloat, n::Integer=1) ?

I think that does not decomplexify; and since nextfloat! is being introduced and not exported .. keeping it as simple as possible lets the exported function be more reliably maintained.

Well, I proposed this change precisely because that seems simpler to me, since default args are a well understood feature, and this allows to merge two definitions into one, with easier maintainability IMHO.

I missed some of your earlier comment. You are talking about giving n the default value of 1. I'd have no objection to that in any of the four functions: nextfloat!, prevfloat!, nextfloat, prevfloat were it not the case that float.jl does not give nextfloat, prevfloat default values. I'm sure that's at least in part because float.jl was written eariler. Nonetheless, there is a proclivity to follow the conventions that are in use.

float.jl does not give nextfloat, prevfloat default values.

My guess would be that it's because those functions are documented separately. By the way, the docstring of nextfloat(x, n) should be updated here to include BigFloat (or simply to remove the type constraint IEEEFloat, which is not even exported or documented).

proclivity

nice new word learnt, thanks!

We can do the same thing here

We can also do it differently, this is what default arguments are for.

Why is it a breaking change -- what is the rounding now?

Currently, we can do setrounding(T, Mode) and the nexfloat() uses that Mode. What is implemented in this PR is RoundNearest Mode.
I would prefer the method mentioned by @rfourquet but would not like to break the code style.

Yes, however when there is no setrounding set, the default is RoundNearest. So its not clear to me that using RoundNearest is breaking anything. If setrounding sets rounding to RoundDown or to RoundUp what difference does it make -- the nextfloat is the same as the nextfloat rounded down and the same as the nextfloat rounded up.
Seems to me that by definition nextfloat(x) != x for finite(x) and similarly for prevfloat .. if they cannot be equal and presuming that nextfloat(prevfloat(x)) == x then it is RoundNearest whatever you call it.

I would prefer the method mentioned by @rfourquet but would not like to break the code style.

I wonder why you would think that this would break code style. It would rather be your current version which breaks style. If there is no speed advantage (which I doubt in this case) to having two separate bodies with repetition, just merge it into one function with default argument.

rfourquet · 2019-03-20T16:52:52Z

This new implementation of nextfloat(::BigInt) seems more useful to me, and I already considered proposing such a change: indeed it's difficult for me to imagine a use-case for nextfloat(x) which doesn't have the same precision as x. On the other hand, the current behavior is the one which is consistent with setprecision, so it's not an obvious choice. Anyway, I say that just to suggest that explicit documentation of this corner case (deviation from the consistency of setprecision) should be added to this PR.

test/mpfr.jl

JeffreySarnoff · 2019-03-22T03:37:07Z

We have reached a meeting of appropriateness (absence of dissent, presence of consent), right? right. @narendrakpatel has been diligent in his incorporation of the best recentest both in test and in src. Let's clear this PR into Base.

rfourquet · 2019-03-22T08:37:01Z

@narendrakpatel has been diligent in his incorporation of the best recentest both in test and in src. Let's clear this PR into Base.

I wish some more tests and documentation like we suggested, and ideally an answer to my comments. But more importantly, regarding the breaking change, an explicit decision should be taken...

JeffreySarnoff · 2019-03-22T14:33:29Z

@narendrakpatel I thought you had added those tests already -- please do. And the docs modification too.
@rfourquet What is the breaking change -- how does the rounding mode break anything if the result of nextfloat prevfloat is isomorphic to computation under RoundNearest whatever the RoundingMode setting?

rfourquet · 2019-03-22T14:46:40Z

What is the breaking change

See my previous post, the current behavior of nextfloat is to respect setprecision, i.e. nextfloat(x) doesn't necessarily create a float with the same precision as x. I am personally in favor of the proposed change, but this is breaking; so it must be decided whether we want this change, and if yes, whether this is a "minor change".

StefanKarpinski · 2019-03-28T18:51:16Z

This looks good to go aside from needing NEWS 😀

JeffreySarnoff · 2019-03-28T18:52:48Z

supergood -- getting all newsy

JeffreySarnoff · 2019-03-28T22:06:01Z

There is a typo and an inoperative link in the News item:

New methods nexfloat(::BigFloat, n::Int) and prevfloat(::BigFloat, n::Int) are
introduced. ([#31310]) ⬳ omitted the 't' in 'nextfloat'

(reworded to conform to the style of related News items)

nextfloat(::BigFloat, n::Int) and prevfloat(::BigFloat, n::Int) have now been added. (#31310)

Be careful to copy the full link into News: in html it looks like this:
(< a href="https://github.com/JuliaLang/julia/issues/31310">#31310</a>)
to appear as this: (#31310)

@narendrakpatel please make correction and then notify me

JeffreySarnoff · 2019-03-29T05:08:16Z

@narendrakpatel On behalf of the other participants, thank you for this awesome first PR.

narendrakpatel · 2019-03-29T05:25:42Z

If this is okay, can this be merged? @StefanKarpinski @simonbyrne

NEWS.md

rfourquet · 2019-03-29T08:30:09Z

NEWS.md

-  * `inv(::Missing)` has now been added and returns `missing` ([#31408]).
+  * `inv(::Missing)` has now been added and returns `missing` ([#31451]).
+
+  * `nextfloat(::BigFloat, n::Int)` and `prevfloat(::BigFloat, n::Int)` have now been added


nitpick, if you happen to re-push before merge: n::Integer

Suggested change

* `nextfloat(::BigFloat, n::Int)` and `prevfloat(::BigFloat, n::Int)` have now been added

* `nextfloat(::BigFloat, n::Integer)` and `prevfloat(::BigFloat, n:: Integer)` methods have been added

@narendrakpatel is in class and will push this modification when he gets home.

or -- just go ahead and make the change -- we all agree

I am back. I will make the changes right away.

do you know how to "squash"? I don't.

For now, I will just use git commit --amend but you can squash commit in the following way:

git rebase -i HEAD~n

replace pick with squash

You don't have to do anything. It can be squashed by the person doing the merge.

(But yes, it can be done by you as well with the appropriate git incantations.)

StefanKarpinski · 2019-03-29T13:37:00Z

Looks good modulo doc suggestions. Please squash when merging.

narendrakpatel · 2019-03-29T14:02:51Z

@StefanKarpinski I made all the changes that are recommended.

NEWS.md

StefanKarpinski · 2019-03-29T15:09:48Z

NEWS.md

@@ -49,6 +52,9 @@ Standard library changes
 * A no-argument construct to `Ptr{T}` has been added which constructs a null pointer ([#30919])
 * `strip` now accepts a function argument in the same manner as `lstrip` and `rstrip` ([#31211])
 * `mktempdir` now accepts a `prefix` keyword argument to customize the file name ([#31230], [#22922])
+* `nextfloat(::BigFloat)` and `prevfloat(::BigFloat)` now returns a value with the same precision
+    as their argument, which means that (in particular) `nextfloat(prevfloat(x)) == x` whereas


Suggested change

as their argument, which means that (in particular) `nextfloat(prevfloat(x)) == x` whereas

as their argument, which means that (in particular) `nextfloat(prevfloat(x)) == x` whereas

StefanKarpinski · 2019-03-29T15:09:57Z

NEWS.md

@@ -49,6 +52,9 @@ Standard library changes
 * A no-argument construct to `Ptr{T}` has been added which constructs a null pointer ([#30919])
 * `strip` now accepts a function argument in the same manner as `lstrip` and `rstrip` ([#31211])
 * `mktempdir` now accepts a `prefix` keyword argument to customize the file name ([#31230], [#22922])
+* `nextfloat(::BigFloat)` and `prevfloat(::BigFloat)` now returns a value with the same precision
+    as their argument, which means that (in particular) `nextfloat(prevfloat(x)) == x` whereas
+    previously this could result in a completely different value with a different precision ([#31310])


Suggested change

previously this could result in a completely different value with a different precision ([#31310])

previously this could result in a completely different value with a different precision ([#31310])

It is just a markdown thing. I had build the docs and checked that it looks fine. Do you still want me to push to these changes?

Probably not worth it, although it does match the way the rest of the file is formatted. You can also just merge the suggested changes through the GitHub UI.

Changed the files.

StefanKarpinski · 2019-03-29T15:10:49Z

Ok, we're converging here. Now it's just the rebase artifact and markdown indentation.

StefanKarpinski · 2019-03-29T19:04:01Z

This is ready to be squash-merged as soon as tests pass. Great work, @narendrakpatel—thanks for sticking it out to the end!

narendrakpatel · 2019-03-31T13:59:07Z

If this is okay, can this be merged? @StefanKarpinski
PS: some tests are failing but the reasons are not related to this issue.

StefanKarpinski · 2019-04-01T19:32:31Z

Exciting! Congrats on the PR, @narendrakpatel!

narendrakpatel force-pushed the nextfloat-prevfloat branch 3 times, most recently from de29df0 to b0a5ccf Compare March 11, 2019 10:13

fredrikekre requested a review from simonbyrne March 11, 2019 14:25

StefanKarpinski reviewed Mar 11, 2019

View reviewed changes

vtjnash added needs decision A decision on this change is needed triage This should be discussed on a triage call labels Mar 11, 2019

simonbyrne reviewed Mar 11, 2019

View reviewed changes

StefanKarpinski mentioned this pull request Mar 14, 2019

mutating APIs for BigInts and BigFloats? #31342

Open

narendrakpatel force-pushed the nextfloat-prevfloat branch from b0a5ccf to b1e5e41 Compare March 14, 2019 21:19

rfourquet added the bignums BigInt and BigFloat label Mar 20, 2019

rfourquet reviewed Mar 20, 2019

View reviewed changes

test/mpfr.jl Show resolved Hide resolved

JuliaLang deleted a comment from JeffreySarnoff Mar 28, 2019

narendrakpatel added 5 commits March 29, 2019 01:06

implement internal function _duplicate

753096c

implement nextfloat, prevfloat, nextfloat(x,n), prevfloat(x,n)

01c0a58

add tests for new implementation of nextfloat/prevfloat

49541cd

update implementation and tests for nextfloat/prevfloat

562567c

update docs for nextfloat/prevfloat

e23d59d

narendrakpatel force-pushed the nextfloat-prevfloat branch from c32800d to 9db7274 Compare March 28, 2019 19:38

rfourquet approved these changes Mar 28, 2019

View reviewed changes

narendrakpatel force-pushed the nextfloat-prevfloat branch from 9db7274 to d43ea31 Compare March 29, 2019 04:41

JeffreySarnoff approved these changes Mar 29, 2019

View reviewed changes

rfourquet reviewed Mar 29, 2019

View reviewed changes

NEWS.md Outdated Show resolved Hide resolved

rfourquet reviewed Mar 29, 2019

View reviewed changes

narendrakpatel force-pushed the nextfloat-prevfloat branch from d43ea31 to 899d69d Compare March 29, 2019 14:00

StefanKarpinski reviewed Mar 29, 2019

View reviewed changes

NEWS.md Show resolved Hide resolved

StefanKarpinski reviewed Mar 29, 2019

View reviewed changes

update news

2d053f9

narendrakpatel force-pushed the nextfloat-prevfloat branch from 899d69d to 2d053f9 Compare March 29, 2019 18:33

rfourquet merged commit 4c2e570 into JuliaLang:master Apr 1, 2019

		end

		nextfloat(x::BigFloat, n::Int) = nextfloat!(BigFloat(x, new=true), n)

	precision::Integer=MPFR.precision(x), new::Bool=false)
	precision::Integer=MPFR.precision(x), new::Bool=precision!=MPFR.precision(x))

	precision == MPFR.precision(x) && !new && return x
	new \|\| return x

	* `nextfloat(::BigFloat, n::Int)` and `prevfloat(::BigFloat, n::Int)` have now been added
	* `nextfloat(::BigFloat, n::Integer)` and `prevfloat(::BigFloat, n:: Integer)` methods have been added

	as their argument, which means that (in particular) `nextfloat(prevfloat(x)) == x` whereas
	as their argument, which means that (in particular) `nextfloat(prevfloat(x)) == x` whereas

	previously this could result in a completely different value with a different precision ([#31310])
	previously this could result in a completely different value with a different precision ([#31310])

Add nextfloat(::BigFloat, n), prevfloat(::BigFloat,n) #31310

Add nextfloat(::BigFloat, n), prevfloat(::BigFloat,n) #31310

Conversation

narendrakpatel commented Mar 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski commented Mar 11, 2019

narendrakpatel commented Mar 11, 2019

StefanKarpinski commented Mar 11, 2019

simonbyrne Mar 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffreySarnoff Mar 14, 2019 • edited Loading

Choose a reason for hiding this comment

JeffreySarnoff Mar 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffreySarnoff Mar 14, 2019 • edited Loading

Choose a reason for hiding this comment

Keno commented Mar 14, 2019

narendrakpatel commented Mar 14, 2019 • edited Loading

JeffreySarnoff commented Mar 14, 2019

JeffreySarnoff commented Mar 14, 2019

Keno commented Mar 14, 2019

JeffreySarnoff commented Mar 14, 2019

JeffreySarnoff commented Mar 15, 2019

narendrakpatel commented Mar 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffreySarnoff Mar 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfourquet commented Mar 20, 2019

JeffreySarnoff commented Mar 22, 2019 • edited Loading

rfourquet commented Mar 22, 2019

JeffreySarnoff commented Mar 22, 2019

rfourquet commented Mar 22, 2019

StefanKarpinski commented Mar 28, 2019

JeffreySarnoff commented Mar 28, 2019

JeffreySarnoff commented Mar 28, 2019 • edited Loading

JeffreySarnoff commented Mar 29, 2019

narendrakpatel commented Mar 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski commented Mar 29, 2019 • edited Loading

narendrakpatel commented Mar 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski commented Mar 29, 2019

StefanKarpinski commented Mar 29, 2019

narendrakpatel commented Mar 31, 2019

StefanKarpinski commented Apr 1, 2019

simonbyrne Mar 11, 2019 •

edited

Loading

JeffreySarnoff Mar 14, 2019 •

edited

Loading

JeffreySarnoff Mar 14, 2019 •

edited

Loading

JeffreySarnoff Mar 14, 2019 •

edited

Loading

narendrakpatel commented Mar 14, 2019 •

edited

Loading

JeffreySarnoff Mar 20, 2019 •

edited

Loading

JeffreySarnoff commented Mar 22, 2019 •

edited

Loading

JeffreySarnoff commented Mar 28, 2019 •

edited

Loading

StefanKarpinski commented Mar 29, 2019 •

edited

Loading