Skip to content

RIP fortran#59

Merged
zingale merged 7 commits intomasterfrom
numba
Jan 3, 2019
Merged

RIP fortran#59
zingale merged 7 commits intomasterfrom
numba

Conversation

@harpolea
Copy link
Copy Markdown
Collaborator

@harpolea harpolea commented Nov 2, 2018

This removes all the Fortran in the code, makes the signatures to some functions a bit more pythonic and updates the benchmarks (which were also failing for the fortran).

@IanHawke
Copy link
Copy Markdown
Contributor

IanHawke commented Nov 3, 2018

Concerned that Github doesn't have a crying emoji.

@harpolea
Copy link
Copy Markdown
Collaborator Author

harpolea commented Nov 3, 2018

😢

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Nov 3, 2018

do we even need mk.sh anymore?

@harpolea
Copy link
Copy Markdown
Collaborator Author

harpolea commented Nov 3, 2018

Not for building, but mk.sh clean can be useful to remove the cached compiled code as Numba doesn't delete it automatically and you can end up with quite a few files in __pycache__ if you change the code a lot. I expect most users won't be changing any of the numba-compiled code, so it probably won't be a problem for them. Alternatively, I could just get rid of the cache=True option - this would add 2-5 seconds of start up time at the start of every run, and I basically only put it there because I'm impatient.

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Nov 3, 2018

I get large-ish errors for the lm_atm bubble:

pyro ...
initializing the bubble problem...
comparing to: lm_atm/tests/lm_bubble_128_0065 
 
variable comparisons:
density              error =        0.01497213509
x-velocity           error =      0.0005998435495
y-velocity           error =      0.0004395507152
eint                 error =        0.03760221844
phi-MAC              error =      9.470132485e-07
phi                  error =      2.655483131e-05
gradp_x              error =       0.002500767966
gradp_y              error =       0.002386601402
ERROR: one or more variables don't agree

need to double check the Fortran

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Nov 4, 2018

okay, so I agree perfectly (numba and Fortran) if I use the stored output on master in lm_atm/tests/ as the comparison. But I don't agree if I compare against the stored output on numba in lm_atm/tests/. Since the numba and Fortran agree perfectly, my guess is that the benchmark on the numba branch is wrong?

@harpolea
Copy link
Copy Markdown
Collaborator Author

harpolea commented Nov 5, 2018

That's weird - I recomputed the benchmarks as I was getting the same small errors for both the numba and Fortran versions. I'll try running again and see if I see the same thing

@harpolea
Copy link
Copy Markdown
Collaborator Author

harpolea commented Nov 5, 2018

I see the reverse of you: I get the same exact error if I use the version in master, but get no errors if I use the version in numba.

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Jan 3, 2019

remind me why the benchmarks differ on this branch compared to master? For example, lm_atm differs as:

lm_atm/tests/lm_bubble_128_0065.h5
 
variable comparisons:
density              error =        0.01497213509
eint                 error =        0.03760221844
gradp_x              error =       0.002500767966
gradp_y              error =       0.002386601402
phi                  error =      2.655483131e-05
phi-MAC              error =      9.470132485e-07
x-velocity           error =      0.0005998435495
y-velocity           error =      0.0004395507152

@harpolea
Copy link
Copy Markdown
Collaborator Author

harpolea commented Jan 3, 2019

The benchmarks varied depending on which machine/version of the compiler was used (especially for the problems which used the multigrid solver). I think the ones on this branch were compiled on xrb, but should probably be compiled on groot/bender instead

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Jan 3, 2019

okay... I'll regenerate them on groot tomorrow. Can you resolve those conflicts?

also, I assume it is convention that numba files start with __ (like __interface.py)?

@harpolea
Copy link
Copy Markdown
Collaborator Author

harpolea commented Jan 3, 2019

Sure, I'll have a look at this tomorrow

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Jan 3, 2019

the conflicts were my fault. Not sure what I did.

I'll regenerate the benchmarks tomorrow and then merge.

@zingale
Copy link
Copy Markdown
Collaborator

zingale commented Jan 3, 2019

I've reset the benchmarks to the master branch and the results agree to roundoff levels. I've also confirmed that the 4th order compressible still converges as 4th order.

@zingale zingale merged commit c16147b into master Jan 3, 2019
@harpolea harpolea deleted the numba branch February 26, 2019 17:50
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.

3 participants