Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[Perl] Updates mapping between PDL and MX types #20852

Merged
merged 2 commits into from
Feb 1, 2022

Conversation

zmughal
Copy link

@zmughal zmughal commented Jan 27, 2022

Description

This removes the hardcoded values and uses PDL::Type instead.

Upgrades minimum PDL dependency to PDL v2.064 which provides
the int8 (PDL: sbyte) type.

Fixes #20851.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

@mxnet-bot
Copy link

Hey @zmughal , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [edge, clang, centos-gpu, windows-cpu, unix-cpu, website, sanity, miscellaneous, centos-cpu, windows-gpu, unix-gpu]


Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@mseth10 mseth10 added the pr-awaiting-testing PR is reviewed and waiting CI build and test label Jan 27, 2022
@zmughal
Copy link
Author

zmughal commented Jan 27, 2022

@sergeykolychev, I opened this PR against the v1.x branch. I was working using the Dockerfile on that branch and noticed that it does not work with whatever is checked out. Instead it runs git clone which will pull the default branch and not git clone -b v1.x.

Is there any interest in using the current checkout for the Dockerfile? I also noticed that https://hub.docker.com/r/mxnet/perl has not been updated in several years.

I also noticed that the CI is not running any particular tests for the Perl module. Edit: I see them now in Jenkins.

@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Jan 27, 2022
@sergeykolychev
Copy link
Contributor

@zmughal zmughal changed the base branch from v1.x to v1.9.x January 27, 2022 07:19
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Jan 27, 2022
@zmughal
Copy link
Author

zmughal commented Jan 27, 2022

@sergeykolychev, I have updated the base branch and put the commits on top of v1.9.x, though I believe there will still be some test failures.

@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Jan 27, 2022
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Jan 27, 2022
This removes the hardcoded values and uses PDL::Type instead.

Upgrades minimum PDL dependency to PDL v2.064 which provides
the int8 (PDL: sbyte) type.

Fix tests by allowing some PDL data alongside Perl scalars.

Fixes <apache#20851>.
@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Jan 28, 2022
@zmughal
Copy link
Author

zmughal commented Jan 29, 2022

I'm investigating the failure of t/test_sparse_ndarray.t. This is due to a change between PDL v2.068_03 and v2.068_04 and showed up now because there was just a new PDL stable release today.

@zmughal
Copy link
Author

zmughal commented Jan 29, 2022

@mxnet-bot run ci [unix-cpu, unix-gpu]

(PDL v2.071 released)

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu, unix-cpu]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Jan 29, 2022
@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu, unix-cpu]

@zmughal
Copy link
Author

zmughal commented Jan 30, 2022

@mxnet-bot run ci [unix-cpu, unix-gpu]

(PDL v2.072 released)

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu, unix-cpu]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-awaiting-review PR is waiting for code review and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Jan 30, 2022
@zmughal
Copy link
Author

zmughal commented Jan 30, 2022

@sergeykolychev, this PR is now ready and the tests pass.

Of note, there are some test failures with PDL v2.068_04 – v2.071 (all released in the last week), but an upgrade to PDL v2.072 (the latest now) fixes all those issues.

@zmughal
Copy link
Author

zmughal commented Feb 1, 2022

@sergeykolychev, is there anything else you need for this PR?

@sergeykolychev sergeykolychev merged commit 58c9368 into apache:v1.9.x Feb 1, 2022
@sergeykolychev
Copy link
Contributor

sergeykolychev commented Feb 1, 2022

@zmughal Merged it, hopefully further PDL development will not break AI::MXNet tests, may I contact you if this happens ?
Btw, is it possible to lock version of PDL for docker file ?

@sergeykolychev sergeykolychev added Perl and removed pr-awaiting-review PR is waiting for code review labels Feb 1, 2022
@zmughal zmughal deleted the 20851-perl-pdl-type-enum branch February 1, 2022 03:02
@zmughal
Copy link
Author

zmughal commented Feb 1, 2022

@sergeykolychev, thank you!

hopefully further PDL development will not break AI::MXNet tests, may I
contact you if this happens ?

I am working on the PDL CI workflows to help prevent future breakage of
downstream soon, so that should be mitigated in the future. And yes, feel free
to ping me for anything about PDL.

Btw, is it possible to lock version of PDL for docker file ?

Yes, to set an exact version, you can use cpanm with @.

A more complete approach is to use Carton to create a snapshot file which
contains all the versions of every dependency.

Another thing that I might be able to help with is packaging MXNet's C++ part
as an Alien::* package so that it can be installed directly using a CPAN
client. It would download from the appropriate branch for the v1 API. Would
you be interested in adding support for that (in addition to building out of
this repository)? I would give you maintainership/upload bits on it and I
would help keep it up to date. And if the MXNet project ever ships pre-built
binaries for certain platforms, I can support that as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants