Skip to content

cuda.core: method/member/property naming audit #1945

@mdboom

Description

@mdboom

Summary

This is an audit of method/member/property naming in the cuda_core API. (A subset of the larger audit work in #1919).

This is a bot-generated list, some of it good, some of it nonsense, and some of it requires some judgement calls.

Instructions for readers of this issue:

  • Let's discuss in the comments below
  • Once things have consensus, I will mark them with a 👍 or 👎. I have taken the liberty of applying my own judgement as a first pass
  • When the change has been merged, the checkbox will get checked

Based on this prompt:


I want to audit the naming of all members of every public class in cuda_core. You can ignore the private classes, I only care about properties and methods exposed through the public API.

  • Check for PEP8 compliance
  • Properties should be nouns
  • Methods should be verbs
  • Identify methods that could be properties (i.e. take no arguments and do minimal calculation)
  • Check for any naming inconsistencies, such as when different english words or abbreviation forms are used for the same concept or datatype
  • Make sure conceptual method pairs are matching, e.g. add/remove, get/set, create/delete

Methods that could be properties

These take no arguments and return a value. Converting to properties would match existing patterns.

  • Buffer.get_ipc_descriptor() → property ipc_descriptor
  • Event.get_ipc_descriptor() → property ipc_descriptor
  • DeviceMemoryResource.get_allocation_handle() → property allocation_handle
  • PinnedMemoryResource.get_allocation_handle() → property allocation_handle
  • Linker.get_error_log() → property error_log
  • Linker.get_info_log() → property info_log
  • GraphDef.nodes() → property nodes
  • GraphDef.edges() → property edges

system subpackage — no-arg methods that could be properties

  • 👍 system.Device.get_auto_boosted_clocks_enabled() → property
  • 👍 system.Device.get_current_clock_event_reasons() → property
  • 👍 system.Device.get_supported_clock_event_reasons() → property
  • 👎 system.Device.get_supported_event_types() → property
  • 👍 system.Device.get_supported_pstates() → property
  • 👍 system.PciInfo.get_pcie_replay_counter() → property
  • 👍 system.PciInfo.get_max_pcie_link_generation() → property
  • 👍 system.PciInfo.get_gpu_max_pcie_link_generation() → property
  • 👍 system.PciInfo.get_max_pcie_link_width() → property
  • 👍 system.PciInfo.get_current_pcie_link_generation() → property
  • 👍 system.PciInfo.get_current_pcie_link_width() → property

Noun-based methods (should be verb-phrased)

Methods named as nouns rather than verbs, making them look like properties.

cuda.core

  • KernelAttributes.max_threads_per_block(device_id) — noun phrase (takes param so can't be property; consider get_ prefix)
  • KernelAttributes.shared_size_bytes(device_id) — same
  • KernelAttributes.const_size_bytes(device_id) — same
  • KernelAttributes.local_size_bytes(device_id) — same
  • KernelAttributes.num_regs(device_id) — same
  • KernelAttributes.ptx_version(device_id) — same
  • KernelAttributes.binary_version(device_id) — same
  • KernelAttributes.cache_mode_ca(device_id) — same
  • KernelAttributes.max_dynamic_shared_size_bytes(device_id) — same
  • KernelAttributes.preferred_shared_memory_carveout(device_id) — same
  • KernelAttributes.cluster_size_must_be_set(device_id) — same
  • KernelAttributes.required_cluster_width(device_id) — same
  • KernelAttributes.required_cluster_height(device_id) — same
  • KernelAttributes.required_cluster_depth(device_id) — same
  • KernelAttributes.non_portable_cluster_size_allowed(device_id) — same
  • KernelAttributes.cluster_scheduling_policy_preference(device_id) — same
  • KernelOccupancy.max_active_blocks_per_multiprocessor() — noun phrase
  • KernelOccupancy.max_potential_block_size() — noun phrase
  • KernelOccupancy.available_dynamic_shared_memory_per_block() — noun phrase
  • KernelOccupancy.max_potential_cluster_size() — noun phrase
  • KernelOccupancy.max_active_clusters() — noun phrase
  • GraphDef.empty() / GraphNode.empty() — adjective, not verb; consider create_empty_node() or add_empty()

cuda.core.system

  • 👍 system.Device.clock(clock_type) — noun; consider get_clock_info()
  • 👍 system.Device.fan(fan) — noun; consider get_fan_info()
  • 👍 system.Temperature.sensor(sensor) — noun; consider get_sensor_reading()
  • 👍 system.Temperature.threshold(type) — noun; consider get_threshold()
  • 👍 system.Temperature.thermal_settings(idx) — noun phrase; consider get_thermal_settings()

PEP 8 / abbreviation violations

  • Stream.is_nonblockingis_non_blocking (PEP 8 snake_case word boundary)
  • 👎 GraphNode.pred / GraphNode.succpredecessors / successors
  • 👎 AllocNode.dptr / FreeNode.dptr / MemsetNode.dptrdevice_pointer or pointer
  • AllocNode.bytesizebyte_size or size
  • 👎 ConditionalNode.cond_typecondition_type
  • 👍 HostCallbackNode.callback_fncallback
  • 👎 LaunchConfig.shmem_sizeshared_memory_size
  • 👎 GraphBuilder.if_cond() / GraphDef.if_cond() / GraphNode.if_cond()if_condition()
  • 👎 GpuDynamicPstatesUtilization.inc_threshold / dec_thresholdincrease_threshold / decrease_threshold
  • 👎 FanInfo.speed_rpmspeed_in_rpm (match "units in name" pattern)
  • InforomInfo.bbx_flush_timeblackbox_flush_time

Naming inconsistencies across the API

close() vs destroy()

Every class uses close() for resource cleanup (Buffer, Stream, Event, Program, Linker, Graph, GraphBuilder, GraphicsResource, etc.) except:

  • 👎 GraphNode.destroy() — sole exception; should be close() for consistency

allocate()/deallocate() vs alloc()/free()

MemoryResource, Device, _MemPool, GraphMemoryResource all use allocate()/deallocate(), but:

  • 👍 GraphDef.alloc()allocate() for consistency
  • 👍 GraphDef.free()deallocate() for consistency
  • 👍 GraphNode.alloc()allocate() for consistency
  • 👍 GraphNode.free()deallocate() for consistency

as_ vs to_ conversion prefix

  • 👎 StridedMemoryView.as_tensor_map() uses as_ while Device.to_system_device() / system.Device.to_cuda_device() use to_ — pick one convention

Shared memory naming inconsistency

Three different naming patterns for the same concept:

  • LaunchConfig.shmem_size vs KernelAttributes.shared_size_bytes / max_dynamic_shared_size_bytes vs DeviceProperties.max_shared_memory_per_block — unify naming

if_cond abbreviation vs full words elsewhere

  • GraphBuilder.create_conditional_handle() (full) alongside GraphBuilder.if_cond() (abbreviated) — inconsistent
  • GraphDef.create_condition() (full) alongside GraphDef.if_cond() (abbreviated) — inconsistent

add_child() vs embed()

  • GraphBuilder.add_child() vs GraphDef.embed() — same concept (embed a child graph) with different names across the two graph-building interfaces

record_event()/wait_event() vs record()/wait()

  • GraphDef.record_event() / GraphDef.wait_event() include type name "event", while Stream.record() / Stream.wait() drop it — inconsistent

FanInfo.set_default_fan_speed()

  • 👍 Redundant "fan" — already on a FanInfo object; consider reset_speed() or set_default_speed()

Mismatched conceptual pairs

  • 👎 get_ipc_descriptor() / from_ipc_descriptor()get_ doesn't pair with from_; consider export_ipc() / from_ipc() or make getter a property
  • 👎 get_allocation_handle() / from_allocation_handle() — same mismatch

Properties named as non-nouns or with confusing boolean polarity

  • 👍 LaunchConfig.cooperative_launch — reads like an action; is_cooperative would be more idiomatic for a boolean
  • Event.is_timing_disabled — negative boolean; is_timing_enabled avoids double-negatives like not event.is_timing_disabled
  • Event.is_sync_busy_waited — grammatically awkward; uses_busy_wait or is_busy_wait_enabled
  • 👍 system.Device.persistence_mode_enabled — missing is_ prefix for boolean; is_persistence_mode_enabled
  • 👍 system.Device.display_mode — returns bool but name doesn't suggest boolean; is_display_connected
  • 👍 system.Device.display_active — returns bool but name doesn't suggest boolean; is_display_active
  • 👍 system.Device.is_c2c_mode_enabled — verbose; is_c2c_enabled

Metadata

Metadata

Assignees

Labels

cuda.coreEverything related to the cuda.core moduleenhancementAny code-related improvements

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions