Skip to content
This repository has been archived by the owner on May 4, 2019. It is now read-only.

Improve operators #119

Merged
merged 2 commits into from
Jul 3, 2016
Merged

Improve operators #119

merged 2 commits into from
Jul 3, 2016

Conversation

nalimilan
Copy link
Member

Introduce a new null_safe_op() function which is used instead of
isbits() to decide whether to take the fast path (without a branch)
or the slow path in lifted operators. This is more correct (and thus
safer), and allows supporting operations on any type. Also add an
extensive set of tests for both paths.

On Julia 0.4, operators for which no functor exist cannot be identified
by null_safe_op(): for these, fall back to the slow path on that version.

This PR introduces cbrt as a new supported operation since it has a special
operator . It also keeps abs(), abs2(), scalarmin() and scalarmax(), which
should probably be deprecated once we have a more general solution for
lifting.

Tests pass on Julia 0.4, but on 0.5 they need JuliaLang/julia#16995 (unrelated tests already fail too).

Fixes #74, #111, #116.

@coveralls
Copy link

coveralls commented Jun 18, 2016

Coverage Status

Coverage decreased (-0.4%) to 79.16% when pulling 9371332 on nl/ops into cf133da on master.

null_safe_op(f::Any, ::Type)::Bool
null_safe_op(f::Any, ::Type, ::Type)::Bool

Returns whether an operation `f` can safely be applied to any value of the passed type(s).
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit: maybe we should distinguish between value and bit pattern? Consider the following type:

immutable OneType
    x::Int

    function OneType()
        new(1)
    end
end

The only legal value of this type is 1, but our uninitialized constructors will generate bit patterns that are not legal values.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should mention "bit pattern" below instead of "domain"? I think "value" is good as a first approximation to give the idea of the requirements.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I've updated the docstring.

@codecov-io
Copy link

codecov-io commented Jun 26, 2016

Current coverage is 81.61%

Merging #119 into master will decrease coverage by 0.41%

@@             master       #119   diff @@
==========================================
  Files            13         13          
  Lines           729        729          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            598        595     -3   
- Misses          131        134     +3   
  Partials          0          0          

Powered by Codecov. Last updated by 383661a...b783922

@coveralls
Copy link

coveralls commented Jun 26, 2016

Coverage Status

Coverage decreased (-9.5%) to 73.684% when pulling 4da3bfa on nl/ops into 61398f3 on master.

@nalimilan nalimilan force-pushed the nl/ops branch 2 times, most recently from 92e3b27 to bb9ba46 Compare July 3, 2016 11:45
nalimilan added 2 commits July 3, 2016 14:10
Introduce a new null_safe_op() function which is used instead of
isbits() to decide whether to take the fast path (without a branch)
or the slow path in lifted operators. This is more correct (and thus
safer), and allows supporting operations on any type. Also add an
extensive set of tests for both paths.

On Julia 0.4, operators for which no functor exist cannot be identified
by null_safe_op(): for these, fall back to the slow path on that version.

This PR introduces cbrt as a new supported operation since it has a special
operator ∛. It also keeps abs(), abs2(), scalarmin() and scalarmax(), which
should probably be deprecated once we have a more general solution for
lifting.
Nullable{Union{}} is always null so we can return null immediately
if one of the arguments is of that type. For unary operators, return
Nullabl(). For binary operators, return Nullable{S}() with S the element
type of the other argument, which carries more information.
@coveralls
Copy link

coveralls commented Jul 3, 2016

Coverage Status

Coverage decreased (-0.4%) to 81.619% when pulling b783922 on nl/ops into 383661a on master.

@nalimilan nalimilan merged commit c0f4d6a into master Jul 3, 2016
@nalimilan nalimilan deleted the nl/ops branch July 3, 2016 12:17
@nalimilan
Copy link
Member Author

With support merged in Julia, tests now pass. Will tag a new release.

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.

5 participants