Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use recipe channels for remote deps. #62

Merged

Conversation

bethune-bryant
Copy link
Member

@bethune-bryant bethune-bryant commented Jun 4, 2021

Checklist before submitting

Description

@npanpaliya added an external channel to the xgboost env file and it wasn't being used during remote dependency retrieval.

use xgboost branch from https://github.com/npanpaliya/xgboost-feedstock/tree/142-update and run validate config, it would fail.

$> open-ce validate config --conda_build_config=./open-ce/envs/conda_build_config.yaml xgboost-env.yaml 
...
PackagesNotFoundError: The following packages are not available from current channels:
  - gcc_linux-64=8.4.0

@npanpaliya
Copy link

npanpaliya commented Jun 7, 2021

@bethune-bryant - There is one more problem in this flow on x86 RH8 with cuda 11.0.
In _create_remote_deps, we do conda search --info <package> -c <channels>. On x86 when package is cudatoolkit 11.0, the conda search returns 4 entries as follows -

(base) [npanpa23@paim4-2 ~]$ conda search cudatoolkit=11.0.* -c defaults -c conda-forge
Loading channels: done
# Name                       Version           Build  Channel
cudatoolkit                   11.0.3      h15472ef_6  conda-forge
cudatoolkit                   11.0.3      h15472ef_7  conda-forge
cudatoolkit                   11.0.3      h15472ef_8  conda-forge
cudatoolkit                 11.0.221      h6bb024c_0  pkgs/main

And we don't take the latest one, we pick up all of these and read the dependencies of all the entries. Ideally we should pick up the newest version which is 11.0.221. I'm not sure if we have encountered such case ever because we have been using a single channel so far.
If we do conda install cudatoolkit=11.0 -c defaults -c conda-forge it picks the latest one 11.0.221 correctly. And that is what we want with conda search too. We need to set/use channel priority kind of thing somewhere.

Similar prob would arise when we will have exact same version on both the channels like grpcio=1.35.*.

@bethune-bryant
Copy link
Member Author

bethune-bryant commented Jun 7, 2021

We actually get the latest package in get_latest_package_info:

def get_latest_package_info(channels, package):
'''
Get the conda package info for the most recent search result.
'''
package_infos = conda_package_info(channels, package)
retval = package_infos[0]
for package_info in package_infos:
if package_info["timestamp"] > retval["timestamp"]:
retval = package_info
return retval

Which is what we use in _create_remote_deps:

seen.add(package_name)
package_info = conda_utils.get_latest_package_info(self._channels + node.channels, package)
dep_graph.add_node(DependencyNode({package}))

But we've been worried that that could cause issues because it is using the timestamp to get the latest, instead of parsing and comparing versions.

Have you seen an issue with this?

It may be good to see if there is a conda API function for comparing versions. I'll look into that.

@npanpaliya
Copy link

Yes, I see a problem with this. On x86 with cudatoolkit 11.0, we take the output of conda search based on timestamp if there are more entries found. In my case, conda-forge's cudatoolkit 11.0.3 is the output of get_latest_package_info, and that package shows dependencies as follows -

dependencies:
  - __glibc >=2.17,<3.0.a0
  - libgcc-ng >=9.3.0
  - libstdcxx-ng >=9.3.0

And __glibc package is not found on any of the channels. And hence validate config fails.

@bethune-bryant
Copy link
Member Author

@npanpaliya
When choosing a package from the search it now uses the following priority:

  1. Most Specific Channel
  2. Latest Version
  3. Largest Build Number
  4. Latest Timestamp

This is closer to what conda is actually doing when installing a package. This should fix the issues you were seeing.

This could still run into issues if there are dependency conflicts. But doing anymore would require more integration with the conda solver, which is something that we may need to look into in the future if it becomes an issue.

@npanpaliya
Copy link

Thanks a lot @bethune-bryant ! It worked for me.

@codecov-commenter
Copy link

Codecov Report

Merging #62 (01b3f5b) into main (b2aaa52) will increase coverage by 0.01%.
The diff coverage is 97.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   96.69%   96.70%   +0.01%     
==========================================
  Files          23       23              
  Lines        1964     1971       +7     
==========================================
+ Hits         1899     1906       +7     
  Misses         65       65              
Flag Coverage Δ
unittests 96.70% <97.56%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
open_ce/conda_utils.py 98.77% <96.97%> (+0.12%) ⬆️
open_ce/build_command.py 95.08% <100.00%> (-0.16%) ⬇️
open_ce/build_tree.py 97.24% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2aaa52...01b3f5b. Read the comment docs.

@bethune-bryant bethune-bryant merged commit e08302e into open-ce:main Jun 8, 2021
@bethune-bryant bethune-bryant deleted the use_recipe_channels_remote branch June 9, 2021 15:31
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.

5 participants