Skip to content

Conversation

@Thorius
Copy link
Contributor

@Thorius Thorius commented Feb 14, 2020

  • Revert back to compile-time values for reduction types but replace const ReductionType declaration with static constexpr ReductionType.
  • Add /permissive- flag when building on Windows to disable MSVC extensions and improve conformance.

@codecov-io
Copy link

codecov-io commented Feb 15, 2020

Codecov Report

Merging #117 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #117   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           7      7           
  Lines          49     49           
=====================================
  Hits           49     49

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 c4fdd99...3b6c702. Read the comment docs.

@Thorius
Copy link
Contributor Author

Thorius commented Feb 15, 2020

I wanted to add /permissive- flag to MSVC builds to catch any standard conformance errors early, but the PyTorch headers appeared to cause issues with this on cl.exe 14.16 so I removed that change and only kept the static constexpr changes.

And example build: https://travis-ci.org/rusty1s/pytorch_scatter/jobs/650788515?utm_medium=notification&utm_source=github_status
Not sure what might cause this, but probably something like Unicode characters in the header or simply a bug in the compiler.

@rusty1s
Copy link
Owner

rusty1s commented Feb 15, 2020

Thank you very much! This looks awesome.

@rusty1s rusty1s merged commit e490d68 into rusty1s:master Feb 15, 2020
@Thorius Thorius deleted the msvc-constexpr-fix branch February 18, 2020 18:21
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.

3 participants