Skip to content

Conversation

@mloubout
Copy link
Contributor

@mloubout mloubout commented Jan 6, 2026

No description provided.

@mloubout mloubout requested a review from JDBetteridge January 6, 2026 03:37
@mloubout mloubout added the arch jitting, archinfo, ... label Jan 6, 2026
@codecov
Copy link

codecov bot commented Jan 6, 2026

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.97%. Comparing base (e8f63c4) to head (8dad3b5).

Files with missing lines Patch % Lines
devito/arch/archinfo.py 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2815      +/-   ##
==========================================
- Coverage   78.98%   78.97%   -0.01%     
==========================================
  Files         248      248              
  Lines       50821    50822       +1     
  Branches     4391     4391              
==========================================
  Hits        40139    40139              
- Misses       9879     9880       +1     
  Partials      803      803              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

# cudart present but no driver detected. Likely isolation
# run such as version check or within a docker build.
return

Copy link
Contributor

Choose a reason for hiding this comment

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

Anti-pattern, fixing...

Copy link
Contributor

@JDBetteridge JDBetteridge Jan 6, 2026

Choose a reason for hiding this comment

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

According to docs only driver can be 0 (NULL is invalid though)

Copy link
Contributor

Choose a reason for hiding this comment

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

We should 100% be warning users though, not blind returning from this function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add the warning yes. It's highly unlikely a user works of see it though this merely only happens when you for example check devito.version within a docker build (which won't show the warning )

Copy link
Contributor

Choose a reason for hiding this comment

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

Check out my changes and see if you agree

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought there were use cases where you might run Devito on a login node without the driver before deploying on a compute node with the driver (in which case warning but not failing seems like a good course of action)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a use case too yes. As of your change I personally prefer the library call and value check in separate statements but that's really not important so can just merge.
FYI was breaking the pro editable install because it was failing to check version during building

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch jitting, archinfo, ...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants