Biogeme #26374
Biogeme #26374
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
| - scipy >=1.7.3 | ||
| - tqdm >=4.64.1 | ||
| - tomlkit >=0.11.5 | ||
| - cythonbiogeme ==1.0.2 |
There was a problem hiding this comment.
This is not present on conda forge, afaik. You should include a recipe for it in this PR also.
| - tqdm >=4.64.1 | ||
| - tomlkit >=0.11.5 | ||
| - cythonbiogeme ==1.0.2 | ||
| - biogeme-optimization ==0.0.5 |
There was a problem hiding this comment.
Also this one. So you need three recipes in this PR.
| home: http://biogeme.epfl.ch | ||
| summary: Estimation and application of discrete choice models | ||
| license: MIT | ||
| license_family: MIT |
There was a problem hiding this comment.
This license does not seem to be MIT: https://github.com/michelbierlaire/biogeme/blob/master/LICENSE
It seems to be a modification of the MIT license. I'm not sure we can actually distribute this package, given the license.
There was a problem hiding this comment.
MIT license:
Copyright (c) <year> <copyright holders>
Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
License found in biogeme:
BIOGEME is distributed free of charge. We ask each user to register to
Biogeme's users group, and to mention explicitly the use of the
package when publishing results, using the following reference:
For BisonBiogeme: Bierlaire, M. (2003). BIOGEME: A free package for
the estimation of discrete choice models , Proceedings of the 3rd
Swiss Transportation Research Conference, Ascona, Switzerland.
For PythonBiogeme: Bierlaire, M. (2016) PythonBiogeme: a short
introduction. Report TRANSP-OR 160706 ,Series on Biogeme. Transport
and Mobility Laboratory, School of Architecture, Civil and
Environmental Engineering, Ecole Polytechnique Fédérale de Lausanne,
Switzerland.
Disclaimer: This software is provided free of charge and "AS IS"
WITHOUT ANY WARRANTY of any kind. The implied warranties of
merchantability, fitness for a particular purpose and non-infringment
are expressly disclaimed. In no event will the author (Michel
Bierlaire) or his employer (EPFL) be liable to any party for any
direct, indirect, special or other consequential damages for any use
of the code including, without limitation, any lost profits, business
interruption, loss of programs or other data on your information
handling system or otherwise, even if we are expressly advised of the
possibility of such damages.
There was a problem hiding this comment.
Remove license and license_family since this is not the MIT license.
|
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/biogeme:
For recipes/biogeme-optimization:
For recipes/cythonbiogeme:
|
|
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/biogeme:
For recipes/cythonbiogeme:
|
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
recipes/cythonbiogeme/meta.yaml
Outdated
| sha256: f139342944c849b47ce5aaa548e68aa179c195de93ab76be78353f4309684d0e | ||
|
|
||
| build: | ||
| number: 3 |
There was a problem hiding this comment.
Build number should start at 0.
| summary: Various optimization algorithms for teaching and research | ||
| license: MIT | ||
| license_family: MIT | ||
| license_file: LICENSE.txt |
There was a problem hiding this comment.
This is the error from the linux build:
ValueError: License file given in about/license_file (/home/conda/staged-recipes-copy/recipes/biogeme-optimization/LICENSE.txt) does not exist in source root dir or in recipe root dir (with meta.yaml)
There was a problem hiding this comment.
There is no license file in the source tarball for this package. In fact, there is no evidence of a license in the repo at all: https://github.com/michelbierlaire/optimization
If there is no license at all, I don't think we can add this to conda forge because no permission to redistribute is given. Maybe you can open an issue on the author's repository and ask if he will grant permission or add an OSS license to the repository.
|
cythonbiogeme osx failure: |
recipes/cythonbiogeme/meta.yaml
Outdated
| summary: C++ part of the Biogeme package | ||
| license: MIT | ||
| license_family: MIT | ||
| license_file: LICENSE.txt |
There was a problem hiding this comment.
Windows build error:
ValueError: License file given in about/license_file (D:\a\1\s\recipes\cythonbiogeme\LICENSE.txt) does not exist in source root dir or in recipe root dir (with meta.yaml)
recipes/cythonbiogeme/meta.yaml
Outdated
| summary: C++ part of the Biogeme package | ||
| license: MIT | ||
| license_family: MIT | ||
| license_file: LICENSE.txt |
There was a problem hiding this comment.
| license_file: LICENSE.txt | |
| license_file: LICENSE |
recipes/cythonbiogeme/meta.yaml
Outdated
|
|
||
| build: | ||
| number: 3 | ||
| script: /bin/rm -f pyproject.toml && {{ PYTHON }} -m pip install . -vv # [unix] |
There was a problem hiding this comment.
| script: /bin/rm -f pyproject.toml && {{ PYTHON }} -m pip install . -vv # [unix] | |
| script: {{ PYTHON }} -m pip install . -vv |
Should be able to keep pyproject.toml and this should run on windows too (at least to start to see if we need a special script.
recipes/cythonbiogeme/meta.yaml
Outdated
| build: | ||
| number: 3 | ||
| script: /bin/rm -f pyproject.toml && {{ PYTHON }} -m pip install . -vv # [unix] | ||
| skip: true # [python_impl == 'pypy'] |
There was a problem hiding this comment.
Remove this line, we'll see if it runs on pypy later.
| - pip | ||
| run: | ||
| - python | ||
| - cython >=0.29.16 |
There was a problem hiding this comment.
Shouldn't need this a run dependency, remove.
There was a problem hiding this comment.
Nevermind I see it in the setup.cfg:
install_requires =
cython >= 0.29.16
pandas >= 1.3.5
Does the package compile things at run time?
|
cythonbiogeme builds for me locally on linux with the suggested changes I made above. |
@moorepants I tried different things. Unfortunately, I cannot compile it for win. Any suggestion in what I should look for? as I am not very familiar with these process, I don't know what is exactly the problem I am facing? E.g., How can I see if the new win compiler we are using is the same as the one used by Michael? |
|
If the patch doesn't apply then you have a malformed patch file. Until you apply the patch we can't see what compiler errors occur. |
Below is what I am getting if I use the patch file you sent me. This: Are these the type of error we are looking for? |
|
The error you copy above is from the latest build and it shows a compile failure with the MSVC compiler (the default for conda forge builds). If you want to use the suggested compile flags in the cythonbiogeme source code, those flags only work with the mingw compiler (which you can enable in the conda forge build). So the flags have to match the compiler you select. We do not have the correct flags for MSVC because the cythonbiogeme author has never successfully built the package with MSVC and does not have suggested flags for it in the source code (or any build instructions). See the related issues posted in the cythonbiogeme github issue tracker. So you have two paths:
I just noticed that you are using a mix of MSVC and mingw compilers in the current build. You will need to make both the c and C+ compilers be from the same compiler toolset (MSVC or mingw). For 1 or 2 you will need to patch the setup.cfg file to have the appropriate flags on windows. That patch file must be a properly formed binary unix patch file. Many of the errors we've seen here are that the patch can't be applied (maybe you are trying to edit it in a text editor?). If the patch applies, then we will see whether it compiles or not and get some info from the compiler errors. So the steps are chose 1 or 2 (not both), make the patch, and then see if it compiles. If you personally use a Windows computer you can try to compile the software on your own computer using this recipe and you can iterate much faster than waiting on the CI in this pull request. I recommend that if you are using Windows. |
|
Use mingw: requirements:
build:
- {{ compiler('c') }} # [not win]
- {{ compiler('m2w64_c') }} # [win]
- {{ compiler('cxx') }} # [not win]
- {{ compiler('m2w64_cxx') }} # [win]
- {{ stdlib("c") }}Use MSVC: requirements:
build:
- {{ compiler('c') }}
- {{ compiler('cxx') }}
- {{ stdlib("c") }} |
My patch files works, but you seem to be editing the patch file and then it breaks. If you want to create a patch file yourself, then you need to do it using the |
|
Looks like you'll need this change too: - {{ stdlib("c") }} # [not win]
- {{ stdlib("m2w64_c") }} # [win] |
I have this now: requirements:
build:
- {{ compiler('c') }} # [not win]
- {{ compiler('m2w64_c') }} # [win]
- {{ compiler('cxx') }} # [not win]
- {{ compiler('m2w64_cxx') }} # [win]
- {{ stdlib("c") }} |
|
You need dual stdlib calls for win and not win. |
|
I noticed that most R recipes (e.g. https://github.com/conda-forge/r-seurat-feedstock/blob/2f3707d1e1d539199051a766ac6645d8a143de9e/recipe/meta.yaml#L3) have: {% set posix = 'm2-' if win else '' %}
{% set native = 'm2w64-' if win else '' %}at the top of the file when using mingw. |
|
But that just looks like a way to install some unix tools: https://github.com/conda-forge/r-seurat-feedstock/blob/2f3707d1e1d539199051a766ac6645d8a143de9e/recipe/meta.yaml#L82 So probably not needed. |
|
Here is a python cython based install that uses mingw: https://github.com/conda-forge/pyreadr-feedstock/blob/6d08f1b553882341a0ec7c14fe9bd198afdbf7c0/recipe/meta.yaml It has a couple things: requirements:
build:
- python # [build_platform != target_platform]
- cross-python_{{ target_platform }} # [build_platform != target_platform]
- cython # [build_platform != target_platform]
- {{ compiler("m2w64_c") }} # [win]
- {{ compiler('c') }} # [not win]
host:
- python
- pip
- cython
- m2w64-toolchain # [win]
- libpython # [win]
- bzip2 # [not win]
- zlib
run:
- python
- pandas
- m2w64-gcc-libs # [win]
- bzip2 # [not win]It is possible we need the |
|
Maybe also helpful from pyreader: https://github.com/ofajardo/pyreadr/blob/master/windows_compilation.md |
|
Something about mingw and pthread (which we need): conda-forge/conda-forge.github.io#2045 maybe relevant. |
|
Gives an example of specifying the winpthreads: https://github.com/conda-forge/r-scattermore-feedstock/blob/0a0a6d33728e6e90ff5723fe746017d3d0b74095/recipe/meta.yaml#L44 |
|
The lastest build is a patch application failure: |
|
Maybe relevant: conda-forge/conda-forge.github.io#1891 |
|
Latest Windows error: Two things:
Two solution paths: 1) figure out how to ensure compilation is with mingw or 2) figure out how to link against pthreads with MSVC. |
I am really not familiar with these topics. I think am acting as guessing. But I have some ideas patching the file to uncomment the setup.cfg marked for windows. So will go now for option 1, as I have no clue for going for option 2. |
|
I recommend asking for help from the conda forge community and the cythonbiogeme author. |
Checklist
url) rather than a repo (e.g.git_url) is used in your recipe (see here for more details).