Skip to content
This repository has been archived by the owner on Sep 21, 2021. It is now read-only.

Test Orthogonal Projections #46

Merged
merged 2 commits into from
Sep 1, 2018
Merged

Conversation

arnavs
Copy link
Member

@arnavs arnavs commented Aug 30, 2018

Not much code in this one. Just verified that the three different projection schemes come out to the same result.

@arnavs arnavs requested a review from Nosferican August 30, 2018 04:41
@arnavs arnavs mentioned this pull request Aug 30, 2018
53 tasks
rst_files/orth_proj.rst Outdated Show resolved Hide resolved
@jlperla
Copy link
Member

jlperla commented Aug 30, 2018

Do we really need all of these atol = 1e-10 everywhere? Isn't the default tolerance enough? I would prefer to drop them if possible.

One thing, though, is that you can never test that something @test x ≈ 0 without an atol as it can't figure out the relative tolerance between the two sides. We will need to discuss that in the notes at some point.

@jlperla
Copy link
Member

jlperla commented Aug 30, 2018

I think we should have using LinearAlgebra at the top of every lecture. Julia without that is pretty confusing for users who start playing with the notebook.

@arnavs
Copy link
Member Author

arnavs commented Aug 30, 2018

@jlperla The default tolerance for isapprox is full machine precision (1e-16), I believe. I put these in as a precautionary measure (maybe we're not printing the full float, maybe there are differences with different machines, etc.). But they could be redundant, and we could probably remove them at some point.

Nevermind, this isn't true:

julia> 1 ≈ 1.0000001
false

julia> 1 ≈ 1.00000001
true

So yeah, we could definitely delete these. Will do from now on.

@Nosferican
Copy link
Collaborator

Having LinearAlgebra at least at this stage will break some lectures given I and a few other namespace conflicts. Might be sensible during the re-write...

@Nosferican
Copy link
Collaborator

I know for some with QuantEcon given a naive (in the lectures) and a high precision one (in the package) I think I needed 1e-6 or 1e-8.

@jlperla
Copy link
Member

jlperla commented Aug 30, 2018

The default tolerance for isapprox is full machine precision (1e-16),

That isn't true. It uses the machine epsilon, but comparing floating points adjusts for the scale. See https://docs.julialang.org/en/v1.0.0/base/math/#Base.isapprox

Try to avoid them if possible and just add in extra numbers if necessary, but not a big deal.

@jlperla
Copy link
Member

jlperla commented Aug 30, 2018

Having LinearAlgebra at least at this stage will break some lectures given I and a few other namespace conflicts. Might be sensible during the re-write...

Sure, it can wait. But it also is only a problem if you use the I as a uniform scaling. See

julia> using LinearAlgebra

julia> I = 5
5

julia> I
5

@arnavs
Copy link
Member Author

arnavs commented Aug 30, 2018

OK, I stripped the tolerances from the tests and there were no issues. @Nosferican is this good to merge?

@Nosferican Nosferican merged commit 51806e7 into master Sep 1, 2018
@arnavs arnavs deleted the Test-Orthogonal_Projections branch September 7, 2018 21:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants