search

Feldspar / feldspar-language

The goal of the Feldspar project is to define a high-level language that allows description of high-performance digital signal processing algorithms.
Other
45 stars 3 forks source link

Move ONNX related files to separate directory. #503

Closed kffaxen closed 3 years ago

kffaxen commented 3 years ago

The changes in this commit are necessary for Cabal to work correctly when onnxToFeld.hs starts using parts of that library. The library is located under 'src' and the ONNX related files were previously in sub directories of 'src'. With that structure the onnxToFeld executable cannot have feldspar-language as a build depend in the Cabal file which would force the entry for that executable to include all of the files and dependencies of the library.

pjonsson commented 3 years ago

Huh? The modules for the library and executable are listed in other-modules and are disjoint and I just did a stack test with build-depends: feldspar-language, <other dependencies> on onnxToFeld and that worked.

kffaxen commented 3 years ago

Things will break once 'onnxToFeld.hs' starts importing modules from the Feldspar hierarchy, such as 'Feldspar.Compiler.Imperative.Frontend'. The thing is that if we want to avoid listing all of the Feldspar modules and build dependencies and instead refer to the Feldspar library as a build-depend of the executable onnxToFeld just as our test suite does, we can not have 'src' in 'hs-source-dirs' of onnxToFeld. If we have, stack/Cabal/ghc will resolve module names from the imports in 'onnxTofeld.hs' as source files rather than as the modules of feldspar-language it just built. And cabal (or ghc) will just build them again. So it should really be this way. The structure we started with was flawed.

pjonsson commented 3 years ago

We discussed where to put Onnx when we imported it and I liked your idea with keeping it under src, just like we have tests under tests and examples under examples.

It looks like GHC gets confused about home modules, whatever that means. Does the following at the top of onnxToFeld.hs:

{-# LANGUAGE PackageImports #-}
{-# LANGUAGE OverloadedStrings #-}
{-# OPTIONS_GHC -Wall #-}

import "feldspar-language" Feldspar.Compiler.Imperative.Frontend

make things work with our current repository layout? If it does, can we use that for now and move the ONNX library when we move onnxToFeld.hs?

As a side note, the order for the file pragmas is language options followed by GHC options. The language options are sorted by length and when two options have equal length, they are sorted in alphabetical order. I'm not sure the logic is great but it's better than the order pragmas were added in and most other files follow it right now.

kffaxen commented 3 years ago

Replaced by the pull request on 'onnx-onnx'.