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

use a separate module for algodiff instead of ndarray directly #476

Merged
merged 7 commits into from
Dec 18, 2019

Conversation

tachukao
Copy link
Member

  • created owl_algodiff_primal_ops.ml to host all the ndarray, linalg, and matrix functions that are used for Algodiff in one place. The main advantage here is that we don't need to inject all the algodiff-required operations in to the ndarray module as was done previously (see Interface files for base/dense and base/linalg #472).

  • refactored owl_algodiff_build_ops.ml. previously I was exposing functions like op_siso, but there really is no need for this. I've put all the relevant functions close to each other so the code is a bit more readable, but it's still lacking in documentation.

Ideally, perhaps in a separate PR, we would have a submodule called Arr in owl_algodiff_primal_ops.D. This module includes all the Ndarray functions. This would involve changing quite a lot of the code . As a concrete example, we would need to change A.float_to_elt to A.Arr.float_to_elt here

https://github.com/tachukao/owl/blob/b52264635cc9d3019c610c272e5baae4f8f3fa0e/src/base/compute/owl_computation_operator.ml#L67

If we make this change, I think it would also make sense to rename the module A to PrimalOps in Algodiff. Conceptually this would make things a lot more consistent. PrimalOps would include submodules Arr, Mat, and Linalg, which are the primal operations that are used for algodiff.

@mseri
Copy link
Member

mseri commented Dec 17, 2019

This looks like a nice cleanup

@ryanrhymes
Copy link
Member

How about creating a algodiff directory in owl like that in base, rather than storing algodiff related files in optimise directory?

@ryanrhymes ryanrhymes added enhancement R&D Core research and development labels Dec 17, 2019
@ryanrhymes ryanrhymes added this to In Progress in owl development via automation Dec 17, 2019
Copy link
Collaborator

@jzstark jzstark left a comment

Choose a reason for hiding this comment

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

Thanks Calvin! This PR separates out the Mat/Linalg functions in Ndarray, which I think is really nice. My only concern is about the idea of low level ndarrys: so now instead of owl_ndarray and owl_base_ndarray, the users also have owl_base_algodiff_primal_ops and owl_base_algodiff_primal_ops to plug in functors such as in CGraph. Practically I think this new mechanism works quite well, but I'm not quite sure if that's a sound design choice.

@mseri
Copy link
Member

mseri commented Dec 17, 2019

Should we also consider, in a separate PR, to drop all the owl_ and to define sublibraries for the various su packages? I’d be happy to give it a try in a week or so

@mseri
Copy link
Member

mseri commented Dec 18, 2019

@jzstark there is only the primal ops module to plumb, or I missed something? I actually like that primal ops include the various submodules separately now

@mseri
Copy link
Member

mseri commented Dec 18, 2019

@tachukao are the examples still working? If not we should update them

@jzstark
Copy link
Collaborator

jzstark commented Dec 18, 2019

Hi Marcello, as I said, I think practically this is a really nice way to separate different modules. My worry is just about the design issue. In Owl the ndarray is meant as the only foundamental data structure to support the layers above, e.g. CGraph, AD, symbolic, etc. There is a consistency of interfaces. Now that we have two sets of such data structure, that might lead to a confusion in using.

That being said, I think this PR has more pros than cons (perhaps the naming of files could be updated? "algdiff_primal_ops" sounds a bit confusing to me...), and I'm happy to accept it.

Btw, I'm not clear about the motivation of dropping the "owl_" headers of all the files.

@tachukao
Copy link
Member Author

How about creating a algodiff directory in owl like that in base, rather than storing algodiff related files in optimise directory?

done! @ryanrhymes

Thanks Calvin! This PR separates out the Mat/Linalg functions in Ndarray, which I think is really nice. My only concern is about the idea of low level ndarrys: so now instead of owl_ndarray and owl_base_ndarray, the users also have owl_base_algodiff_primal_ops and owl_base_algodiff_primal_ops to plug in functors such as in CGraph. Practically I think this new mechanism works quite well, but I'm not quite sure if that's a sound design choice.

@jzstark owl_algodiff_primal_ops is the only interface that satisfies Owl_types_ndarray_algodiff.Sig now. Owl_dense_ndarray_s.ml no longer works.

@tachukao
Copy link
Member Author

@tachukao are the examples still working? If not we should update them

@mseri as far as I'm aware, the examples never actually make and Algodiff module. They simply use the made Algodiff.D or Algodiff.S modules which work fine. But maybe @ryanrhymes can confirm this?

@tachukao
Copy link
Member Author

tachukao commented Dec 18, 2019

That being said, I think this PR has more pros than cons (perhaps the naming of files could be updated? "algdiff_primal_ops" sounds a bit confusing to me...), and I'm happy to accept it.

@jzstark @mseri @ryanrhymes any suggestions for naming? open to new ideas...not entirely happy with the name. Briefly considered owl_algodiff_pops, where pops stands for primal ops...

@mseri
Copy link
Member

mseri commented Dec 18, 2019

I think practically this is a really nice way to separate different modules. My worry is just about the design issue. In Owl the ndarray is meant as the only foundamental data structure to support the layers above, e.g. CGraph, AD, symbolic, etc. There is a consistency of interfaces. Now that we have two sets of such data structure, that might lead to a confusion in using.

That being said, I think this PR has more pros than cons (perhaps the naming of files could be updated? "algdiff_primal_ops" sounds a bit confusing to me...), and I'm happy to accept it.

Sure. I wanted to understand better your concerns. I think this is the time and place to try and figure out the correct design. We should not rush the merge For me personally it would be useful to see a couple of nontrivial examples before and after this PR

@mseri
Copy link
Member

mseri commented Dec 18, 2019

Btw, I'm not clear about the motivation of dropping the "owl_" headers of all the files.

Sorry, I mean that we are doing namespacing manually instead of letting dune deal with it. But it is OT here now
I’ll move the discussion and explain myself properly in a new issue in the next few days. Let’s not clutter this PR

@ryanrhymes
Copy link
Member

That being said, I think this PR has more pros than cons (perhaps the naming of files could be updated? "algdiff_primal_ops" sounds a bit confusing to me...), and I'm happy to accept it.

@jzstark @mseri @ryanrhymes any suggestions for naming? open to new ideas...not entirely happy with the name. Briefly considered owl_algodiff_pops, where pops stands for primal ops...

Owl engine in Owl Symbolic also needs a similar module, we can also name them consistently e.g. like owl_ndarray_symbolic, owl_ndarray_algodiff, so we know they are stub modules to provide matching module signature. But we can discuss this naming thing later in another PR.

@ryanrhymes ryanrhymes merged commit 5379375 into owlbarn:master Dec 18, 2019
@mseri
Copy link
Member

mseri commented Dec 18, 2019

Why not waiting for the renaming before merging?

@ryanrhymes ryanrhymes moved this from In Progress to Done (2019) in owl development Dec 19, 2019
ryanrhymes added a commit that referenced this pull request Dec 23, 2019
mseri added a commit to mseri/opam-repository that referenced this pull request Feb 25, 2020
CHANGES:

*  Fix bug in _squeeze_broadcast (owlbarn/owl#503)
*  Added the Dawson function (Ndarray + Matrix + Algodiff op) (owlbarn/owl#502)
*  Fix bug in reverse mode gradients of aiso operations and pow (owlbarn/owl#501)
*  Added poisson_rvs to Owl_distribution (owlbarn/owl#499)
*  Draw poisson RVs in Ndarray and Mat modules (owlbarn/owl#498)
*  Broadcast bug for higher order derivatives (owlbarn/owl#495)
*  add sem to dense ndarray and matrix (owlbarn/owl#497)
*  Avoid input duplication with Graph.model and multi-input nn (owlbarn/owl#494)
*  Added Graph.get_subnetwork for constructing subnetworks (owlbarn/owl#491)
*  Make Graph.inputs give unique names to inputs (owlbarn/owl#493)
*  modify nlp interfaces
*  Re-add removed DiffSharp acknowledgment (owlbarn/owl#486)
*  add pretty printer for hypothesis type
*  update lambda neuron (owlbarn/owl#485)
*  fix example due to owlbarn/owl#476
*  Extend base linalg functions to complex numbers (owlbarn/owl#479)
*  [breaking] use a separate module for algodiff instead of ndarray directly (owlbarn/owl#476)
*  temp workaround and unittest (owlbarn/owl#478)
*  [breaking] Interface files for base/dense and base/linalg (owlbarn/owl#472)
*  Port code to dune2 (owlbarn/owl#474)
*  [breaking]  interface files to simplify .mli files in owl/dense (owlbarn/owl#471)
*  Save and load Npy files (owlbarn/owl#470)
*  Owl: relax bounds on base and stdio (owlbarn/owl#469)
*  Merged tests for different AD operations into one big test + autoformat tests with ocamlformat (owlbarn/owl#468)

### 0.7.2 (2019-12-06)

* fourth order finite diff approx to grad
* changes to improve stability of sylv/discrete_lyap
* fix bug in concatenate function
* add mli for owl_base_linalg_generic
* Owl-base linalg routines: LU decomposition  (owlbarn/owl#465)
* bug fixes
* Update owl.opam

### 0.7.1 (2019-11-27)

* Add unit basis
* Fix issue owlbarn/owl#337 and owlbarn/owl#457 (owlbarn/owl#458)
* owl-base: drop seemingly unnecessary dependency on integers (owlbarn/owl#456)

### 0.7.0 (2019-11-14)

* Add unsafe network save (owlbarn/owl#429)
* Sketch Count-Min and Heavy-Hitters
* Various bugfixes
* Owl_io.marshal_to_file: use to_channel
* Do not create .owl folder when loading owl library
* Re-design of exceptions and replace asserts with verify
* Add OWL_DISABLE_LAPACKE_LINKING_FLAG
* Reorganise Algodiff module
* Add parameter support to Zoo
* Two new features in algodiff: eye and linsolve (triangular option) + improved stability of qr and chol
* Implemented solve triangular
* Added linsolve and lq reverse-mode differentiation
* Fix build on archlinux (pkg-config cblas)
* Add median and sort along in ndarray
* Improve stability of lyapunov gradient tests

### 0.6.0 (2019-07-17)

* Add unsafe network save (owlbarn/owl#429)
* Sketch Count-Min and Heavy-Hitters
* Various ugfixes
* Owl_io.marshal_to_file: use to_channel
* Do not create .owl folder when loading owl library
* Re-design of exceptions and replace asserts with verify
* Add OWL_DISABLE_LAPACKE_LINKING_FLAG
* Reorganise Algodiff module
* Add parameter support to Zoo
* Two new features in algodiff: eye and linsolve (triangular option) + improved stability of qr and chol
* Implemented solve triangular
* Added linsolve and lq reverse-mode differentiation
* Fix build on archlinux (pkg-config cblas)
* Add median and sort along in ndarray
* Improve stability of lyapunov gradient tests

### 0.5.0 (2019-03-05)

* Improve building and installation.
* Fix bugs and improve performance.
* Add more functions to Algodiff.
* Split plot module out as sub library.
* Split Tfgraph module out as sub library.

### 0.4.2 (2018-11-10)

* Optimise computation graph module.
* Add some core math functions.
* Fix bugs and improve performance.

### 0.4.1 (2018-11-01)

* Improve the APIs of Dataframe module.
* Add more functions in Utils module.

### 0.4.0 (2018-08-08)

* Fix some bugs and improve performance.
* Introduce computation graph into the functor stack.
* Optimise repeat and tile function in the core.
* Adjust the OpenCL library according to computation graph.
* Improve the API of Dataframe module.
* Add more implementation of convolution operations.
* Add dilated convolution functions.
* Add transposed convolution functions.
* Add more neurons into the Neural module.
* Add more unit tests for core functions.
* Move from `jbuilder` to `dune`
* Assuage many warnings

### 0.3.8 (2018-05-22)

* Add initial support for dataframe functionality.
* Add IO module for Owl's specific file operations.
* Add more helper functions in Array module in Base.
* Add core functions such as one_hot, slide, and etc.
* Fix normalisation neuron in neural network module.
* Fix building, installation, and publishing on OPAM.
* Fix broadcasting issue in Algodiff module.
* Support negative axises in some ndarray functions.
* Add more statistical distribution functions.
* Add another higher level wrapper for CBLAS module.

### 0.3.7 (2018-04-25)

* Fix some bugs and improve performance.
* Fix some docker files for automatic image building.
* Move more pure OCaml implementation to base library.
* Add a new math module to support complex numbers.
* Improve the configuration and building system.
* Improve the automatic documentation building system.
* Change template code into C header files.
* Add initial support for OpenMP with evaluation.
* Tidy up packaging using TOPKG.

### 0.3.6 (2018-03-22)

* Fix some bugs and improve performance.
* Add more unit tests for Ndarray module.
* Add version control in Zoo gist system.
* Add tensor contract operations in Ndarray.
* Add more documentation of various functions.
* Add support of complex numbers of convolution and pooling functions.
* Enhance Owl's own Array submodule in Utils module.
* Fix pretty printer for rank 0 ndarrays.
* Add functions to iterate slices in an ndarray.
* Adjust the structure of OpenCL module.

### 0.3.5 (2018-03-12)

* Add functions for numerical integration.
* Add functions for interoperation.
* Add several root-finding algorithms.
* Introduce Owl's exception module.
* Add more functions in Linalg module.
* Add native convolution function in core.
* Remove Eigen dependency of dense data structure.
* Fix some bugs and improve performance.

### 0.3.4 (2018-02-26)

* Update README, ACKNOWLEDGEMENT, and etc.
* Initial implementation of OpenCL Context module.
* Fix some bugs and improve the performance.
* Add Adam learning rate algorithm in Optimise module.
* Add a number of statistical functions into Stats.
* Enhance View functor and add more functions.
* Include and documentation of NLP modules.
* Add dockerfile for various linux distributions.

### 0.3.3 (2018-02-12)

* Fix some bugs and improve the performance.
* Integrate with Owl's documentation system.
* Add native C implementation of pooling operations.
* Add more operators in Operator module.
* Add more functions in Linalg module.
* Optimise the Base library.
* Add more unit tests.

### 0.3.2 (2018-02-08)

* Fix some bugs and improve the performance.
* Functorise many unit tests and add more tests.
* Rewrite the documentation migrate to Sphinx system.
* Migrate many pure OCaml code into Base library.
* Implement the initial version of Base library.

### 0.3.1 (2018-01-25)

* Design View module as an experimental module for Ndarray.
* Include Mersenne Twister (SFMT) to generate random numbers.
* Implement random number generator of various distributions.
* Implement native functions for maths and stats module.
* Include FFTPACK to provide native support for FFT functions.
* Minimise dependency, remove dependencies on Gsl and etc.
* Implement slicing and indexing as native C functions.
* Use new extended indexing operators for slicing functions.
* Refine ndarray fold function and introduce scan function.
* Reorganise the module structure in the source tree.
* Fix some bugs and enhance the performance of core functions.
* Add another 200+ unit tests.

### 0.3.0 (2017-12-05)

* Migrate to jbuilder building system.
* Unify Dense Ndarray and Matrix types.
* Split Toplevel out as a separate library.
* Redesign Zoo system for recursive importing.
* Simplify the module signature for Ndarray.
* Introduce functions in Ndarray module to support in-place modification.
* Introduce reduction functions to reduce an ndarray to a scalar value.
* Add Lazy functor to support lazy evaluation, dataflow, and incremental computing.
* Implement a new and more powerful pretty printer to support both ndarray and matrix.
* Fix bugs in the core module, improve the performance.

### 0.2.8 (2017-09-02)

* New Linalg module is implemented.
* New neural network module supports both single and double precision.
* New Optimise and Regression module is built atop of Algodiff.
* Experimental Zoo system is introduced as a separate library.
* Enhance core functions and fix some bugs.

### 0.2.7 (2017-07-11)

* Enhance basic math operations for complex numbers.
* Redesign Plot module and add more plotting functions.
* Add more hypothesis test functions in Owl.Stats module.
* Support both numerical and algorithmic differentiation in Algodiff.
* Support both matrices and n-dimensional arrays in Algodiff module.
* Support interoperation of different number types in Ext and new operators.
* Support more flexible slicing, tile, repeat, and concatenate functions.
* Support n-dimensional array of any types in Dense.Ndarray.Any module.
* Support simple feedforward and convolutional neural networks.
* Support experimental distributed and parallel computing.

### 0.2.0 (2017-01-20)

* Support both dense and sparse matrices.
* Support both dense and sparse n-dimensional arrays.
* Support both real and complex numbers.
* Support both single and double precisions.
* Add more vectorised operation: sin, cos, ceil, and etc.
* Add basic unit test framework for Owl.
* Add a couple of Topic modelling algorithms.

### 0.1.0 (2016-11-09)

* Initial architecture of Owl library.
* Basic support for double precision real dense matrices.
* Basic linear functions for dense matrices.
* Basic support for plotting functions.
* SI, MKS, CGS, and CGSM metric system.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement R&D Core research and development
Projects
No open projects
owl development
  
Done (2019)
Development

Successfully merging this pull request may close these issues.

None yet

4 participants