search

martibosch / pylandstats

Computing landscape metrics in the Python ecosystem
https://doi.org/10.1371/journal.pone.0225734
GNU General Public License v3.0
82 stars 16 forks source link

Transonic0.4 #2

Closed paugier closed 4 years ago

paugier commented 4 years ago

Numba is used for Windows and Pythran for Linux and macOS...

paugier commented 4 years ago

The decrease of the coverage is understandable. Transonic replaces the function by the Pythranized function, so the Python function is no longer executed.

I think we need to rerun a test testing this function with the environment variable TRANSONIC_NO_REPLACE set. Do you know which test(s) should be rerun like this ? We don't need to run all the tests twice just for this...

paugier commented 4 years ago

I forgot to add Numba as runtime dependency for Windows! I'll have to modify the commit.

martibosch commented 4 years ago

Hello @paugier

thank you very much for your PR. I have added a simple test with that ensures that the Landscape._adjacency_df property (which uses the transonic-decorated function) also works when 'TRANSONIC_NO_REPLACE` is set. Nevertheless, this does not solve the decrease in test coverage. It seems that the latter could be improved by testing the numba backend somehow. The rest (the linux part) seems to be working properly, so if you have any idea about how to test the backend part, we can add it and merge it. Otherwise, such a decrease in test coverage is not a big deal.

Once this is merged, I will see whether it can be conda-build PyLandStats in Windows.

PD: do you have any academic citation for transonic? I would gladly add it to the article for PyLandStats, which is currently undergoing minor revisions. Martí

paugier commented 4 years ago

Ah It's nearly fine now! There is only one line which is not tested (backend = "numba") since it corresponds to Windows. I think tests on Windows cannot be run with Travis so it seems ok to me...

Now the only citation for Transonic is the article on FluidDyn (the broader project from which Transonic was created) : https://fluiddyn.readthedocs.io/en/latest/#metapaper-and-citation

We'll have to write a proper article on Transonic later ! By the way, good luck for your article!

Once this is merged, I will see whether it can be conda-build PyLandStats in Windows.

I'm very interested to see that! I think it should work! Tell me if you encounter problems. Do not forget the runtime dependency on numba (only for Windows of course).

martibosch commented 4 years ago

I'm very interested to see that! I think it should work! Tell me if you encounter problems. Do not forget the runtime dependency on numba (only for Windows of course).

isn't adding the numba runtime dependency in windows what these lines do?

or are you reminding me about the numba runtime dependency for the conda build?

Thank you again!

paugier commented 4 years ago

or are you reminding me about the numba runtime dependency for the conda build?

Yes! In the conda-forge recipe

martibosch commented 4 years ago

Eurkea! https://github.com/conda-forge/pylandstats-feedstock/pull/8

I will add an acknowledgement (and link to transonic) in the README as well as a citation to FluidDyn in PyLandStats' academic paper. Thank you again @paugier

paugier commented 4 years ago

Great news that it now works with conda-forge also on Windows!

Tell me if you or your users encounter any problems related to Transonic.

CC @serge-sans-paille : Packages using Pythran via Transonic can easily switch to Numba (or Cython) for the modules that can not be compiled by Pythran on Windows.

martibosch commented 4 months ago

Hello @paugier,

I am planning another v3.0.0 rc (probably the last one before the final v3.0.0) once pythran#2203 is solved, and since the release includes many performance improvements, I was wondering if it would be possible to use the "pythran" backend for Windows as well. Is this currently supported in transonic?

Thank you. Best, Martí

paugier commented 4 months ago

Yes I guess you can now build wheels on Windows. This is supported by Transonic.

For fluiddyn packages using Pythran (for example https://foss.heptapod.net/fluiddyn/fluidimage), we recently move to the Meson build system (to follow Scipy and scikit-image) and I'm happy with it. I can try to open a PR on pylandstats with such changes. Tell me if you would be interested.

martibosch commented 4 months ago

Hello @paugier!

I have never used Meson - I have gone through some blog posts about its pros/cons but I do not have much expertise in including non-Python extensions to Python packages. I trust your criteria and I could certainly use this opportunity to learn more about it. Therefore, a PR would certainly be very welcome. Otherwise, let me know and I will try to switch to using pythran on windows (by modifying the current setup.py).

Thanks in advance. Best, Martí

martibosch commented 3 months ago

just a note so that I/we remember to use pythran>=0.16.1 - see serge-sans-paille/pythran/pull/2212.