Skip to content
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

Move function and symbol Value implementations => their respective namespaces #184

Merged
merged 110 commits into from
Dec 8, 2020

Conversation

sritchie
Copy link
Member

@sritchie sritchie commented Dec 6, 2020

This PR:

  • moves the Symbol value implementation into sicmutils.abstract.number
  • adds explicit Value implementations for the various function types, and moves these (and the arity code) to sicmutils.function
  • more generic function implementations for ::v/function
  • lots more generative tests for functions, numbers
  • adds clearer Approximate implementations for our various types, so all numbers now compare appropriately to complex and each other with ish?

Plus a couple small fixes.

@@ -157,14 +251,6 @@
returns a fn that applies them in reverse order. Like a curried andThen (the
reverse of compose)"))))

(deftest string-form-test
Copy link
Member Author

Choose a reason for hiding this comment

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

this moved to the simplifier tests.

(g/+ (s/up 4 3 1) v))
(deferred v))
"Slightly tougher since this works with structures")))
(checking "invert" 100 [n sg/real]
Copy link
Member Author

Choose a reason for hiding this comment

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

these are tests of all the new generic methods implemented for fns.

(is (= 33 (add+mul 2 3 4)))
(is (= -15 (add-mul 2 3 4)))
(is (= 15 (mul-add 2 3 4)))))))

(deftest operators
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to operator, since "operator" doesn't "exist yet", as far as the function implementation is concerned.

@@ -61,6 +63,9 @@
long
integer]))

(def real
Copy link
Member Author

Choose a reason for hiding this comment

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

this came in from a previous PR.

@@ -81,15 +86,57 @@
d)]
(r/rationalize n d))))

(def ^:dynamic *complex-tolerance* 1e-12)
(defn square-matrix
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved this in from matrix_test.cljc so we could start consolidating our generators.

(gen/fmap #(apply m/by-rows %)
(gen/vector (gen/vector entry-gen n) n))))

(defn- int=? [this that]
Copy link
Member Author

Choose a reason for hiding this comment

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

this and the following protocol extension allow all of our real types to compare approximately with complex.

Copy link
Collaborator

Choose a reason for hiding this comment

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

what does int= mean? Is one of these guaranteed to be complex?

@@ -237,62 +231,88 @@
(is (c/complex? (g/asin 2)))
(is (c/complex? (g/acos 2))))

(testing "sin"
Copy link
Member Author

Choose a reason for hiding this comment

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

woohoo, all of these are now generative tests.

Copy link
Collaborator

@littleredcomputer littleredcomputer left a comment

Choose a reason for hiding this comment

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

cool, but that seems like a lot of kerfuffle for complex

@@ -140,6 +136,16 @@
::native-integral
::floating-point)))

#?(:clj Boolean :cljs boolean)
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't that the very definition of a bike shed. I think the answer is not obvious (I could argue that the correct answer is "the identity function"), while the stakes are vanishingly low

(v/real? that) (si/*comparator*
(u/double this)
(u/double that))
:else (= this that)))

(extend-protocol si/Approximate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ya learn something new every day!

Copy link
Member Author

Choose a reason for hiding this comment

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

The same library has been great... this idea comes up in every damned library. Why can't a language give it to us out of the box??

(gen/fmap #(apply m/by-rows %)
(gen/vector (gen/vector entry-gen n) n))))

(defn- int=? [this that]
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does int= mean? Is one of these guaranteed to be complex?

@sritchie sritchie changed the base branch from sritchie/rename_value to master December 8, 2020 12:40
@sritchie sritchie merged commit 86ccc7b into master Dec 8, 2020
@sritchie sritchie deleted the sritchie/move_value_definitions branch December 8, 2020 13:00
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.

None yet

3 participants