[WIP] Add native amos #52
Conversation
|
Perhaps the script that did the conversion should be committed as well? Not sure how much it matters but perhaps |
ViralBShah
force-pushed
the
native-amos
branch
from
55952c7 to
8981416
Compare
|
The script no longer works. It is julia 0.3, I think. I had to edit a couple of small things in the generated code. The script and the original fortran and julia codes are on the Compile times are pretty ok. I think it adds maybe 5 seconds to |
|
There isn't much in the way of array access here, and given that the time matches the fortran times, I suspect not much is to be gained from |
src/amos/dgamln.jl
Outdated
| ZP::Float64 = zero(Float64) | ||
| ZSQ::Float64 = zero(Float64) | ||
| begin | ||
| GLN[Int32(1)] = 0.0 |
There was a problem hiding this comment.
These should be moved outside the function.
|
This is pretty unreadable stuff. Is there any hope of maintaining or understanding this if we import undocumented, autotranslated code like this? |
|
To be clear, I like the idea of native Julia implementations of these kinds of things, but I fear we're really going to shoot ourself in the foot here in the long run by losing all the comments and knowledge embedded in the existing Amos codebase. I argue the Fortran code of e.g. |
|
I think it is about as unreadable as the original fortran code. The library has not changed in ages, so there is no upstream benefit. |
|
We could insert URLs in the auto-translated code to the original fortran codebase. I would say that this julia translation has received more eyeballs already than the original fortran code (in context of those of us maintaining this). |
|
If we could maintain the original comments, I think that would be enough to give us a hope of maintaining/debugging this. How did you generate this? |
|
This was already pre-generated by @nolta - See my comments above. Getting the comments in would mean updating the parser code. Extracting the top level comments and sticking those in should be easy with some script. But, if we want to retain all the inline comments as well, that may need to get the original parser code working again - perhaps in an older version of Julia. That may be non-trivial effort. |
|
I agree that the comments are probably necessary for this to be maintainable. It would also be nice if we could replace some of the |
|
I am happy to try and get the comments in. I don't think I want to muck too much further about the If there is general agreement about merging this with comments, I would do it. But I don't wish to spend much more time than that. |
|
Why not just see this as a "compiled" version without any hope maintaining it i.e the code is the Fortran code and this is the compilation target? |
|
Because if that's the case we should just compile the Fortran code. It also
sounds like the "compilation process" is somewhat involved; e.g. if we need
to change something we can't just change the Fortran code and then
"rebuild" the Julia code; there are manual steps that must be taken.
On Tue, Sep 12, 2017, 11:51 Kristoffer Carlsson ***@***.***> wrote:
Why not just see this as a "compiled" version without any hope maintaining
it i.e the code is the Fortran code and this is the compilation target?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#52 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAH_aBhntzZLjxEsMdOw8OLsxsBagL-Oks5shtKVgaJpZM4PSWzK>
.
--
…-E
|
|
There has been no change to the fortran code in years. I am not sure I understand why we want to hang on to it. In case we want to change the fortran code, we can directly edit the julia code going forward. In any case, I don't think that openspecfun is a good library to have. It is an accidental library with stuff we have stuck in that did not have a good natural place otherwise. There is very little chance it will receive any meaningful contributions. Perhaps it doesn't need it. However, I do see that people are not very comfortable with this direction, and I will drop this effort. |
|
I still think we should do the translation, just that it should be maintainable (which I acknowledge is probably more work than is possible at the moment). |
|
Let's keep this A good GSoC project for next year might be to reimplement these algorithms (and perhaps Faddeeva as well) natively in Julia, as was done this year with |
|
Yes, if we want a maintainable Julia code, we should write Julian implementations, rather than tweaking the code translator here. Agree could be a good GSoC project. |
Julia native Amos implementation. Passes all tests and should be as fast as the fortran implementation.