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

[NDTensors] UnallocatedArrays and UnspecifiedTypes #1213

Merged
merged 188 commits into from
Jan 11, 2024

Conversation

kmp5VT
Copy link
Collaborator

@kmp5VT kmp5VT commented Oct 18, 2023

Description

In this PR I add a FieldType folders where I implement a number of abstract, lazy and unspecified immutable fields which will later be implemented into NDTensors.

Checklist:

  • Implement UnallocatedArrays and UnspecifiedTypes
  • Create a SetParameter system for the UnallocatedArrays types
  • Create rudimentary unittests for these new types

As a recap of our conversation 11/20. I will be working on the following

  • Remove AbstractUnallocatedArray
  • Try to make functions instance off AbstractFill and keep UnallocatedZeros a AbstractZero.
  • Write the discussed constructor UnallocatedX(f::X, alloc::Type{<:AbstractArray}) where X is fill or zero. And have set_alloctype call this function.
  • Create arithmetic that preserves the Unallocated type
    • addition and subtraction preserve types
  • create full unittest suite for these types

@kmp5VT kmp5VT requested a review from mtfishman October 18, 2023 18:20
@kmp5VT kmp5VT marked this pull request as draft October 18, 2023 18:20
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4638f7c) 84.03% compared to head (7924683) 53.93%.

❗ Current head 7924683 differs from pull request most recent head f8ed013. Consider uploading reports for the commit f8ed013 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1213       +/-   ##
===========================================
- Coverage   84.03%   53.93%   -30.10%     
===========================================
  Files         100       99        -1     
  Lines        8544     8491       -53     
===========================================
- Hits         7180     4580     -2600     
- Misses       1364     3911     +2547     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mtfishman
Copy link
Member

I think we could split this into two modules, say UnallocatedArrays and UnspecifiedTypes.

Also note that JuliaArrays/FillArrays.jl#299 is merged so we can use AbstractZeros and AbstractOnes.

@mtfishman mtfishman changed the title Kmp5/feature/field types [NDTensors] UnallocatedArrays and UnspecifiedTypes Oct 19, 2023
@kmp5VT
Copy link
Collaborator Author

kmp5VT commented Jan 9, 2024

@mtfishman It looks like theres an issue on Github where it's having trouble using the internet to download/check the deps. This causes tests to prematurely crash/fail. I have been restarting them here and on Main

@mtfishman
Copy link
Member

Looks good to me, thanks!

@mtfishman mtfishman merged commit 54fbb6c into ITensor:main Jan 11, 2024
7 of 8 checks passed
@kmp5VT
Copy link
Collaborator Author

kmp5VT commented Jan 11, 2024

@mtfishman It looks like Jenkins is failing for ITensorGPU with Julia v 1.6 but I cannot replicate the error. When I look at the error thrown by Jenkins, i.e.

failed: [-0.0009661297423744132, 0.0005560180954419236, -0.0010679817640976697, 0.0005560181043973156, -0.0009661297481295442] ≈ [-0.0009661297423742309, 0.000556018095441646, -0.0010679817589570595, 0.0005560180924686908, -0.0009661297199891616]

The biggest difference in this vector is ~2e-11 which i guess is above the rtol for doubles. Do you have any suggestions for this issue?

@mtfishman
Copy link
Member

Increase rtol a bit? Floating point results can be different on different systems.

@kmp5VT
Copy link
Collaborator Author

kmp5VT commented Jan 11, 2024

@mtfishman It doesn't look like it had any issue on main. All jenkins ITensorGPU tests have already passed.

@mtfishman
Copy link
Member

Ok, if you saw it happen it may be worth doing anyway since it may mean rtol is just slightly too small to be robust to roundoff.

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.

4 participants