fix: raise clear ValueError for unsupported MemoryUnit - #1336

This PR is a mirror pull request created from fix: raise clear ValueError for unsupported MemoryUnit to allow the code to access PyDyna CICD secrets.

Check the original PR made by @guhou-hvi for more details.

Closes #1334

Original pull request

fix: raise clear ValueError for unsupported MemoryUnit

Summary

BaseRunner.get_memory_string() raises a bare KeyError for any MemoryUnit other than MB/GB. This PR replaces that with a clear ValueError matching what the enum docstring already promises.

Root cause

MemoryUnit in src/ansys/dyna/core/run/options.py defines five values:

class MemoryUnit:
    """Memory unit for LS-DYNA command line.  Only MB and GB are currently supported."""

    BYTE = 0
    KB = 1
    MB = 2
    GB = 3
    TB = 4

But get_memory_string() only maps two of them:

unit = {MemoryUnit.MB: "m", MemoryUnit.GB: "G"}[self.memory_unit]

Passing MemoryUnit.BYTE, KB, or TB (or any other value) raises:

KeyError: 1

MemoryUnit is a plain class of ints, so the message is just the raw integer — it doesn't mention which enum member was used, doesn't list what's supported, and doesn't align with the docstring's "Only MB and GB are currently supported" promise. All three concrete runners (LinuxRunner, WindowsRunner, DockerRunner) call self.get_memory_string() while building the command line, so the failure surfaces late and looks like an internal lookup error.

Changes

src/ansys/dyna/core/run/base_runner.py:

• Hoist the supported units map into a local units dict.

• Check membership explicitly and raise ValueError with the offending value and the supported names:

  f"Unsupported memory unit: {self.memory_unit}. "
  f"Only MemoryUnit.MB and MemoryUnit.GB are supported."

• Return f"{self.memory}{units[self.memory_unit]}" unchanged for supported units.

tests/test_base_runner.py (new):

• test_memory_string_supported – parameterised over MB → "20m" and GB → "20G".
• test_memory_string_unsupported_raises_value_error – parameterised over BYTE, KB, TB; asserts ValueError with the supported-units message.
• test_memory_string_error_mentions_offending_value – checks the message includes the value the caller passed.
• test_memory_string_default_is_mb – pins the default from BaseRunner.__init__.

The new tests are pure mocks (no solver, no Docker, no filesystem) and deliberately do not carry pytest.mark.run, so they execute in the default unit test matrix (pytest -m "not codegen and not run" in CI) rather than only in the solver-backed job.

Testing

The two test_localrun.py failures are pre-existing environmental issues on this machine, unrelated to this change:

• test_find_solver_with_version – expects Ansys 231, host has 221.
• test_find_solver_latest_install – Linux-style path separator (/) asserted against Windows os.path.join output (\).

Impact

Callers that today pass MemoryUnit.BYTE/KB/TB will receive a ValueError instead of a KeyError. ValueError is a subclass of Exception, so any code catching the general Exception still works; code specifically catching KeyError (unlikely — the raw message gives no hint that this is a supported-units issue) would need to update. Default (MemoryUnit.MB) and MemoryUnit.GB behaviour is unchanged.

Scope

Only BaseRunner.get_memory_string() changes. No modification to MemoryUnit itself, the three concrete runners, or any docstring in this PR. Related local_solver.py follow-up (env var robustness) is being tracked separately and does not conflict.