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

Improve basic datatypes #1756

Merged
merged 345 commits into from
Mar 18, 2024
Merged

Improve basic datatypes #1756

merged 345 commits into from
Mar 18, 2024

Conversation

EmilyBourne
Copy link
Member

@EmilyBourne EmilyBourne commented Feb 20, 2024

This PR rewrites the datatyping system. Fixes #1729.
As described in the issue, the existing superclass Datatype is replaced with 2 superclasses representing slightly different concepts:

  • PrimitiveType : Representing the category of datatype (integer/floating point/etc)
  • PyccelType : Representing the actual type of the object in Python

Subclasses of PyccelType generally fall into one of two categories:

  • FixedSizeType
  • ContainerType

These types are described in developer_docs/type_inference.md (it is recommended to read this file first before reviewing this pull request.

The precision is removed from ast.basic.TypedAstNode and is stored in the FixedSizeTypes where it is relevant. This allows us to remove the necessity for negative precision values as defaults, as the types can now be easily differentiated using the different FixedSizeTypes.
The attribute _dtype is removed from ast.basic.TypedAstNode instances. It is still accessible via the dtype property but is extracted from the class type. This ensures that the two do not deviate from one another.

The NumPy type deduction is significantly improved due to two major improvements. Firstly an assertion is added to NumpyNDArrayType to ensure that the internal datatypes are NumPy types. This uncovered a few bugs where the native Python types were used in our current code. Thus the wrong type would be returned if an element of one of these objects was returned.
Secondly np.result_type is now used to deduce the result type of NumPy functions. This function is implemented by NumPy which guarantees that it handles all corner cases and matches the expected behaviour of NumPy. Fixes #1763.

The CustomDatatype is simplified slightly to remove the unused parameters.

The printing of floats/complexes in Fortran is improved to stop printing decimal digits beyond the precision of the number. Additionally printing the return value of functions returning multiple results of different types is fixed.

Unfortunately these changes have a huge effect on the code as datatypes appear almost everywhere. I have tried my best to find all the places where the code has changed however it is possible that some places were missed if coverage is missing.

@EmilyBourne
Copy link
Member Author

/bot run linux

@pyccel-bot
Copy link

pyccel-bot bot commented Feb 20, 2024

Hello again! Thank you for this new pull request 🤩.

Please begin by requesting your checklist using the command /bot checklist

@Farouk-Echaref
Copy link
Contributor

/bot run linux

@Farouk-Echaref
Copy link
Contributor

/bot run linux

@EmilyBourne EmilyBourne added the Ready_for_review Received at least one approval. Requires review from senior developer label Mar 16, 2024
@EmilyBourne
Copy link
Member Author

/bot run pr_tests

Copy link

@pyccel-bot pyccel-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job ! Your PR is using all the code it added/changed.

@pyccel-bot pyccel-bot bot removed the Ready_for_review Received at least one approval. Requires review from senior developer label Mar 17, 2024
@pyccel-bot
Copy link

pyccel-bot bot commented Mar 17, 2024

@yguclu, @EmilyBourne has been working hard and thinks that they have now replied to or fixed all your comments. Could you take another look at the PR and see if you can approve now?

@pyccel-bot pyccel-bot bot added the Ready_for_review Received at least one approval. Requires review from senior developer label Mar 17, 2024
@pyccel-bot pyccel-bot bot requested a review from yguclu March 17, 2024 10:47
@pyccel-bot pyccel-bot bot removed the Ready_for_review Received at least one approval. Requires review from senior developer label Mar 17, 2024
@pyccel-bot
Copy link

pyccel-bot bot commented Mar 17, 2024

@yguclu, @EmilyBourne has been working hard and thinks that they have now replied to or fixed all your comments. Could you take another look at the PR and see if you can approve now?

@pyccel-bot pyccel-bot bot added the Ready_for_review Received at least one approval. Requires review from senior developer label Mar 17, 2024
@pyccel-bot pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed Ready_for_review Received at least one approval. Requires review from senior developer labels Mar 17, 2024
@yguclu
Copy link
Member

yguclu commented Mar 17, 2024

/bot run pr_tests

@yguclu
Copy link
Member

yguclu commented Mar 17, 2024

/bot show tests

@pyccel-bot
Copy link

pyccel-bot bot commented Mar 17, 2024

The following is a list of keywords which can be used to run tests. Tests in bold are run by pull requests when they are marked as ready for review:

  • linux : Runs the unit tests on a Linux system.
  • windows : Runs the unit tests on a Windows system.
  • macosx : Runs the unit tests on a MacOS X system.
  • coverage : Runs the unit tests on a Linux system and checks the coverage of the tests.
  • docs : Checks if the documentation follows the numpydoc format.
  • pylint : Runs pylint on files which are too big to be handled by codacy.
  • pyccel_lint : Runs a linter to check that Pyccel's best practices are followed.
  • spelling : Checks if everything in the documentation is spelled (and capitalised) correctly.
  • pr_tests : Runs all the tests marked in bold.
  • pickle : Checks that .pyccel files have been correctly generated and installed by the installation process.
  • editable_pickle : Checks that .pyccel files have been correctly generated and installed by the editable installation process.
  • pickle_wheel : Checks that .pyccel files have been correctly generated and packaged into the wheel.
  • anaconda_linux : Runs the unit tests on a linux system using anaconda for python.
  • anaconda_windows : Runs the unit tests on a windows system using anaconda for python.
  • intel : Runs the unit tests on a linux system using the intel compiler.

These tests can be run with the command /bot run X (multiple tests can be specified separated by spaces), or with try V X to test on Python version V.

@yguclu
Copy link
Member

yguclu commented Mar 17, 2024

/bot run coverage

1 similar comment
@EmilyBourne
Copy link
Member Author

/bot run coverage

Copy link

@pyccel-bot pyccel-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job ! Your PR is using all the code it added/changed.

@pyccel-bot pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed Ready_to_merge Approved by senior developer. Ready for final approval and merge labels Mar 18, 2024
@yguclu yguclu merged commit 718e1f3 into devel Mar 18, 2024
15 checks passed
@yguclu yguclu deleted the devel-issue1729 branch March 18, 2024 11:05
EmilyBourne added a commit that referenced this pull request Apr 9, 2024
Add missing cast when creating an array from an object with a different
datatype. Fixes #1785

In order to call the cast on the argument passed to `np.array` (which
may be a `InhomogeneousTupleType`) fix the type of the cast functions.

Modify `InhomogeneousTupleType.datatype` to return a `FixedSizeType` if
the datatypes of all elements are equivalent.

Also fix some minor bugs after #1756
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internals Pyccel's internal behavior, does not affect user experience Ready_to_merge Approved by senior developer. Ready for final approval and merge Type specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Result type of numpy.sum Improve basic datatypes
6 participants