Unverified Commit a4d3e0c4 authored by Kelly Guo's avatar Kelly Guo Committed by GitHub

Switches unittest to pytest for testing (#2459)

# Description

Switches our unit testing framework from unittest to pytest to enable
better CI logging and improve developer experience when checking for
test failures.

## Type of change

- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing
functionality to not work as expected)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [ ] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

<!--
As you go through the checklist above, you can mark something as done by
putting an x character in it

For example,
- [x] I have done this task
- [ ] I have not done this task
-->

---------
Co-authored-by: 's avatarPascal Roth <roth.pascal@outlook.de>
Co-authored-by: 's avatarPascal Roth <57946385+pascal-roth@users.noreply.github.com>
parent 93fd2120
This diff is collapsed.
......@@ -63,3 +63,6 @@ _build
# Teleop Recorded Dataset
datasets
# Tests
tests/
......@@ -33,6 +33,25 @@
"args" : ["--task", "Isaac-Reach-Franka-v0", "--num_envs", "32"],
"program": "${workspaceFolder}/scripts/reinforcement_learning/rsl_rl/play.py",
"console": "integratedTerminal"
},
{
"name": "Python: SinglePytest",
"type": "python",
"request": "launch",
"module": "pytest",
"args": [
"${file}"
],
"console": "integratedTerminal"
},
{
"name": "Python: ALL Pytest",
"type": "python",
"request": "launch",
"module": "pytest",
"args": ["source/isaaclab/test"],
"console": "integratedTerminal",
"justMyCode": false
}
]
}
......@@ -511,7 +511,7 @@ if "%arg%"=="-i" (
set "skip=1"
)
)
!python_exe! tools\run_all_tests.py !allArgs!
!python_exe! -m pytest tools !allArgs!
goto :end
) else if "%arg%"=="--test" (
rem run the python provided by Isaac Sim
......@@ -525,7 +525,7 @@ if "%arg%"=="-i" (
set "skip=1"
)
)
!python_exe! tools\run_all_tests.py !allArgs!
!python_exe! -m pytest tools !allArgs!
goto :end
) else if "%arg%"=="-v" (
rem update the vscode settings
......
......@@ -390,7 +390,7 @@ while [[ $# -gt 0 ]]; do
# run the python provided by isaacsim
python_exe=$(extract_python_exe)
shift # past argument
${python_exe} ${ISAACLAB_PATH}/tools/run_all_tests.py $@
${python_exe} -m pytest ${ISAACLAB_PATH}/tools $@
# exit neatly
break
;;
......
......@@ -13,4 +13,3 @@ These include:
"""
from .app_launcher import AppLauncher # noqa: F401, F403
from .runners import run_tests # noqa: F401, F403
# Copyright (c) 2022-2025, The Isaac Lab Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
"""Sub-module with runners to simplify running main via unittest."""
import unittest
def run_tests(verbosity: int = 2, **kwargs):
"""Wrapper for running tests via ``unittest``.
Args:
verbosity: Verbosity level for the test runner.
**kwargs: Additional arguments to pass to the `unittest.main` function.
"""
# run main
unittest.main(verbosity=verbosity, exit=True, **kwargs)
......@@ -39,6 +39,10 @@ INSTALL_REQUIRES = [
"pillow==11.0.0",
# livestream
"starlette==0.46.0",
# testing
"pytest",
"pytest-mock",
"junitparser",
"flatdict==4.0.1",
]
......
......@@ -4,44 +4,39 @@
# SPDX-License-Identifier: BSD-3-Clause
import argparse
import unittest
from unittest import mock
from isaaclab.app import AppLauncher, run_tests
class TestAppLauncher(unittest.TestCase):
"""Test launching of the simulation app using AppLauncher."""
@mock.patch("argparse.ArgumentParser.parse_args", return_value=argparse.Namespace(livestream=1))
def test_livestream_launch_with_argparser(self, mock_args):
"""Test launching with argparser arguments."""
# create argparser
parser = argparse.ArgumentParser()
# add app launcher arguments
AppLauncher.add_app_launcher_args(parser)
# check that argparser has the mandatory arguments
for name in AppLauncher._APPLAUNCHER_CFG_INFO:
self.assertTrue(parser._option_string_actions[f"--{name}"])
# parse args
mock_args = parser.parse_args()
# everything defaults to None
app = AppLauncher(mock_args).app
# import settings
import carb
# acquire settings interface
carb_settings_iface = carb.settings.get_settings()
# check settings
# -- no-gui mode
self.assertEqual(carb_settings_iface.get("/app/window/enabled"), False)
# -- livestream
self.assertEqual(carb_settings_iface.get("/app/livestream/enabled"), True)
# close the app on exit
app.close()
if __name__ == "__main__":
run_tests()
import pytest
from isaaclab.app import AppLauncher
@pytest.mark.usefixtures("mocker")
def test_livestream_launch_with_argparser(mocker):
"""Test launching with argparser arguments."""
# Mock the parse_args method
mocker.patch("argparse.ArgumentParser.parse_args", return_value=argparse.Namespace(livestream=1, headless=True))
# create argparser
parser = argparse.ArgumentParser()
# add app launcher arguments
AppLauncher.add_app_launcher_args(parser)
# check that argparser has the mandatory arguments
for name in AppLauncher._APPLAUNCHER_CFG_INFO:
assert parser._option_string_actions[f"--{name}"]
# parse args
mock_args = parser.parse_args()
# everything defaults to None
app = AppLauncher(mock_args).app
# import settings
import carb
# acquire settings interface
carb_settings_iface = carb.settings.get_settings()
# check settings
# -- no-gui mode
assert carb_settings_iface.get("/app/window/enabled") is False
# -- livestream
assert carb_settings_iface.get("/app/livestream/enabled") is True
# close the app on exit
app.close()
......@@ -4,35 +4,30 @@
# SPDX-License-Identifier: BSD-3-Clause
import os
import unittest
from isaaclab.app import AppLauncher, run_tests
import pytest
from isaaclab.app import AppLauncher
class TestAppLauncher(unittest.TestCase):
"""Test launching of the simulation app using AppLauncher."""
def test_livestream_launch_with_env_var(self):
"""Test launching with no-keyword args but environment variables."""
# manually set the settings as well to make sure they are set correctly
os.environ["LIVESTREAM"] = "1"
# everything defaults to None
app = AppLauncher().app
@pytest.mark.usefixtures("mocker")
def test_livestream_launch_with_env_vars(mocker):
"""Test launching with environment variables."""
# Mock the environment variables
mocker.patch.dict(os.environ, {"LIVESTREAM": "1", "HEADLESS": "1"})
# everything defaults to None
app = AppLauncher().app
# import settings
import carb
# import settings
import carb
# acquire settings interface
carb_settings_iface = carb.settings.get_settings()
# check settings
# -- no-gui mode
self.assertEqual(carb_settings_iface.get("/app/window/enabled"), False)
# -- livestream
self.assertEqual(carb_settings_iface.get("/app/livestream/enabled"), True)
# acquire settings interface
carb_settings_iface = carb.settings.get_settings()
# check settings
# -- no-gui mode
assert carb_settings_iface.get("/app/window/enabled") is False
# -- livestream
assert carb_settings_iface.get("/app/livestream/enabled") is True
# close the app on exit
app.close()
if __name__ == "__main__":
run_tests()
# close the app on exit
app.close()
......@@ -3,33 +3,27 @@
#
# SPDX-License-Identifier: BSD-3-Clause
import unittest
import pytest
from isaaclab.app import AppLauncher, run_tests
from isaaclab.app import AppLauncher
class TestAppLauncher(unittest.TestCase):
"""Test launching of the simulation app using AppLauncher."""
@pytest.mark.usefixtures("mocker")
def test_livestream_launch_with_kwargs(mocker):
"""Test launching with keyword arguments."""
# everything defaults to None
app = AppLauncher(headless=True, livestream=1).app
def test_livestream_launch_with_kwarg(self):
"""Test launching with headless and livestreaming arguments."""
# everything defaults to None
app = AppLauncher(headless=True, livestream=1).app
# import settings
import carb
# import settings
import carb
# acquire settings interface
carb_settings_iface = carb.settings.get_settings()
# check settings
# -- no-gui mode
assert carb_settings_iface.get("/app/window/enabled") is False
# -- livestream
assert carb_settings_iface.get("/app/livestream/enabled") is True
# acquire settings interface
carb_settings_iface = carb.settings.get_settings()
# check settings
# -- no-gui mode
self.assertEqual(carb_settings_iface.get("/app/window/enabled"), False)
# -- livestream
self.assertEqual(carb_settings_iface.get("/app/livestream/enabled"), True)
# close the app on exit
app.close()
if __name__ == "__main__":
run_tests()
# close the app on exit
app.close()
This source diff could not be displayed because it is too large. You can view the blob instead.
......@@ -11,7 +11,7 @@ import sys
if sys.platform != "win32":
import pinocchio # noqa: F401
from isaaclab.app import AppLauncher, run_tests
from isaaclab.app import AppLauncher
# launch omniverse app
simulation_app = AppLauncher(headless=True).app
......@@ -198,7 +198,3 @@ class TestPinkIKController(unittest.TestCase):
self.right_hand_roll_link_pose[2] -= 0.05
env.close()
if __name__ == "__main__":
run_tests()
......@@ -11,65 +11,49 @@ warnings.filterwarnings("ignore", category=DeprecationWarning)
import numpy as np
import scipy.interpolate as interpolate
import unittest
from isaaclab.app import run_tests
class TestScipyOperations(unittest.TestCase):
"""Tests for assuring scipy related operations used in Isaac Lab."""
def test_interpolation(self):
"""Test scipy interpolation 2D method."""
# parameters
size = (10.0, 12.0)
horizontal_scale = 0.1
vertical_scale = 0.005
downsampled_scale = 0.2
noise_range = (-0.02, 0.1)
noise_step = 0.02
# switch parameters to discrete units
# -- horizontal scale
width_pixels = int(size[0] / horizontal_scale)
length_pixels = int(size[1] / horizontal_scale)
# -- downsampled scale
width_downsampled = int(size[0] / downsampled_scale)
length_downsampled = int(size[1] / downsampled_scale)
# -- height
height_min = int(noise_range[0] / vertical_scale)
height_max = int(noise_range[1] / vertical_scale)
height_step = int(noise_step / vertical_scale)
# create range of heights possible
height_range = np.arange(height_min, height_max + height_step, height_step)
# sample heights randomly from the range along a grid
height_field_downsampled = np.random.choice(height_range, size=(width_downsampled, length_downsampled))
# create interpolation function for the sampled heights
x = np.linspace(0, size[0] * horizontal_scale, width_downsampled)
y = np.linspace(0, size[1] * horizontal_scale, length_downsampled)
# interpolate the sampled heights to obtain the height field
x_upsampled = np.linspace(0, size[0] * horizontal_scale, width_pixels)
y_upsampled = np.linspace(0, size[1] * horizontal_scale, length_pixels)
# -- method 1: interp2d (this will be deprecated in the future 1.12 release)
func_interp2d = interpolate.interp2d(y, x, height_field_downsampled, kind="cubic")
z_upsampled_interp2d = func_interp2d(y_upsampled, x_upsampled)
# -- method 2: RectBivariateSpline (alternate to interp2d)
func_RectBiVariate = interpolate.RectBivariateSpline(x, y, height_field_downsampled)
z_upsampled_RectBivariant = func_RectBiVariate(x_upsampled, y_upsampled)
# -- method 3: RegularGridInterpolator (recommended from scipy but slow!)
# Ref: https://github.com/scipy/scipy/issues/18010
func_RegularGridInterpolator = interpolate.RegularGridInterpolator(
(x, y), height_field_downsampled, method="cubic"
)
xx_upsampled, yy_upsampled = np.meshgrid(x_upsampled, y_upsampled, indexing="ij", sparse=True)
z_upsampled_RegularGridInterpolator = func_RegularGridInterpolator((xx_upsampled, yy_upsampled))
# check if the interpolated height field is the same as the sampled height field
np.testing.assert_allclose(z_upsampled_interp2d, z_upsampled_RectBivariant, atol=1e-14)
np.testing.assert_allclose(z_upsampled_RectBivariant, z_upsampled_RegularGridInterpolator, atol=1e-14)
np.testing.assert_allclose(z_upsampled_RegularGridInterpolator, z_upsampled_interp2d, atol=1e-14)
if __name__ == "__main__":
run_tests()
def test_interpolation():
"""Test scipy interpolation 2D method."""
# parameters
size = (10.0, 12.0)
horizontal_scale = 0.1
vertical_scale = 0.005
downsampled_scale = 0.2
noise_range = (-0.02, 0.1)
noise_step = 0.02
# switch parameters to discrete units
# -- horizontal scale
width_pixels = int(size[0] / horizontal_scale)
length_pixels = int(size[1] / horizontal_scale)
# -- downsampled scale
width_downsampled = int(size[0] / downsampled_scale)
length_downsampled = int(size[1] / downsampled_scale)
# -- height
height_min = int(noise_range[0] / vertical_scale)
height_max = int(noise_range[1] / vertical_scale)
height_step = int(noise_step / vertical_scale)
# create range of heights possible
height_range = np.arange(height_min, height_max + height_step, height_step)
# sample heights randomly from the range along a grid
height_field_downsampled = np.random.choice(height_range, size=(width_downsampled, length_downsampled))
# create interpolation function for the sampled heights
x = np.linspace(0, size[0] * horizontal_scale, width_downsampled)
y = np.linspace(0, size[1] * horizontal_scale, length_downsampled)
# interpolate the sampled heights to obtain the height field
x_upsampled = np.linspace(0, size[0] * horizontal_scale, width_pixels)
y_upsampled = np.linspace(0, size[1] * horizontal_scale, length_pixels)
# -- method 1: RegularGridInterpolator (replacing deprecated interp2d)
func_RegularGridInterpolator = interpolate.RegularGridInterpolator((x, y), height_field_downsampled, method="cubic")
xx_upsampled, yy_upsampled = np.meshgrid(x_upsampled, y_upsampled, indexing="ij", sparse=True)
z_upsampled_RegularGridInterpolator = func_RegularGridInterpolator((xx_upsampled, yy_upsampled))
# -- method 2: RectBivariateSpline (alternate to interp2d)
func_RectBiVariate = interpolate.RectBivariateSpline(x, y, height_field_downsampled)
z_upsampled_RectBivariant = func_RectBiVariate(x_upsampled, y_upsampled)
# check if the interpolated height field is the same as the sampled height field
np.testing.assert_allclose(z_upsampled_RegularGridInterpolator, z_upsampled_RectBivariant, atol=1e-14)
np.testing.assert_allclose(z_upsampled_RectBivariant, z_upsampled_RegularGridInterpolator, atol=1e-14)
np.testing.assert_allclose(z_upsampled_RegularGridInterpolator, z_upsampled_RegularGridInterpolator, atol=1e-14)
This diff is collapsed.
......@@ -8,7 +8,7 @@
from __future__ import annotations
from isaaclab.app import AppLauncher, run_tests
from isaaclab.app import AppLauncher
# Can set this to False to see the GUI for debugging.
HEADLESS = True
......@@ -138,7 +138,3 @@ class TestOpenXRDevice(unittest.TestCase):
device_1.reset()
device_2.reset()
env.close()
if __name__ == "__main__":
run_tests()
......@@ -5,11 +5,10 @@
"""Launch Isaac Sim Simulator first."""
from isaaclab.app import AppLauncher, run_tests
from isaaclab.app import AppLauncher
# launch the simulator
app_launcher = AppLauncher(headless=True)
simulation_app = app_launcher.app
simulation_app = AppLauncher(headless=True).app
"""Rest everything follows."""
......@@ -18,11 +17,11 @@ import gymnasium as gym
import shutil
import tempfile
import torch
import unittest
import uuid
import carb
import omni.usd
import pytest
from isaaclab.envs.mdp.recorders.recorders_cfg import ActionStateRecorderManagerCfg
......@@ -30,94 +29,88 @@ import isaaclab_tasks # noqa: F401
from isaaclab_tasks.utils.parse_cfg import parse_env_cfg
class TestActionStateRecorderManagerCfg(unittest.TestCase):
"""Test cases for ActionStateRecorderManagerCfg recorder terms."""
@classmethod
def setUpClass(cls):
# this flag is necessary to prevent a bug where the simulation gets stuck randomly when running the
# test on many environments.
carb_settings_iface = carb.settings.get_settings()
carb_settings_iface.set_bool("/physics/cooking/ujitsoCollisionCooking", False)
def setUp(self):
# create a temporary directory to store the test datasets
self.temp_dir = tempfile.mkdtemp()
def tearDown(self):
# delete the temporary directory after the test
shutil.rmtree(self.temp_dir)
def test_action_state_reocrder_terms(self):
"""Check action state recorder terms."""
for task_name in ["Isaac-Lift-Cube-Franka-v0"]:
for device in ["cuda:0", "cpu"]:
for num_envs in [1, 2]:
with self.subTest(task_name=task_name, device=device):
omni.usd.get_context().new_stage()
dummy_dataset_filename = f"{uuid.uuid4()}.hdf5"
# parse configuration
env_cfg = parse_env_cfg(task_name, device=device, num_envs=num_envs)
# set recorder configurations for this test
env_cfg.recorders: ActionStateRecorderManagerCfg = ActionStateRecorderManagerCfg()
env_cfg.recorders.dataset_export_dir_path = self.temp_dir
env_cfg.recorders.dataset_filename = dummy_dataset_filename
# create environment
env = gym.make(task_name, cfg=env_cfg)
# reset all environment instances to trigger post-reset recorder callbacks
env.reset()
self.check_initial_state_recorder_term(env)
# reset only one environment that is not the first one
env.unwrapped.reset(env_ids=torch.tensor([num_envs - 1], device=env.unwrapped.device))
self.check_initial_state_recorder_term(env)
# close the environment
env.close()
def check_initial_state_recorder_term(self, env):
"""Check values recorded by the initial state recorder terms.
Args:
env: Environment instance.
"""
current_state = env.unwrapped.scene.get_state(is_relative=True)
for env_id in range(env.unwrapped.num_envs):
recorded_initial_state = env.unwrapped.recorder_manager.get_episode(env_id).get_initial_state()
are_states_equal, output_log = self.compare_states(recorded_initial_state, current_state, env_id)
self.assertTrue(are_states_equal, msg=output_log)
def compare_states(self, compared_state, ground_truth_state, ground_truth_env_id) -> (bool, str):
"""Compare a state with the given ground_truth.
Args:
compared_state: State to be compared.
ground_truth_state: Ground truth state.
ground_truth_env_id: Index of the environment in the ground_truth states to be compared.
Returns:
bool: True if states match, False otherwise.
str: Error log if states don't match.
"""
for asset_type in ["articulation", "rigid_object"]:
for asset_name in ground_truth_state[asset_type].keys():
for state_name in ground_truth_state[asset_type][asset_name].keys():
runtime_asset_state = ground_truth_state[asset_type][asset_name][state_name][ground_truth_env_id]
dataset_asset_state = compared_state[asset_type][asset_name][state_name][0]
if len(dataset_asset_state) != len(runtime_asset_state):
return False, f"State shape of {state_name} for asset {asset_name} don't match"
for i in range(len(dataset_asset_state)):
if abs(dataset_asset_state[i] - runtime_asset_state[i]) > 0.01:
return (
False,
f'State ["{asset_type}"]["{asset_name}"]["{state_name}"][{i}] don\'t match\r\n',
)
return True, ""
if __name__ == "__main__":
run_tests()
@pytest.fixture(scope="session", autouse=True)
def setup_carb_settings():
"""Set up carb settings to prevent simulation getting stuck."""
carb_settings_iface = carb.settings.get_settings()
carb_settings_iface.set_bool("/physics/cooking/ujitsoCollisionCooking", False)
@pytest.fixture
def temp_dir():
"""Create a temporary directory for test datasets."""
temp_dir = tempfile.mkdtemp()
yield temp_dir
shutil.rmtree(temp_dir)
def compare_states(compared_state, ground_truth_state, ground_truth_env_id) -> tuple[bool, str]:
"""Compare a state with the given ground_truth.
Args:
compared_state: State to be compared.
ground_truth_state: Ground truth state.
ground_truth_env_id: Index of the environment in the ground_truth states to be compared.
Returns:
bool: True if states match, False otherwise.
str: Error log if states don't match.
"""
for asset_type in ["articulation", "rigid_object"]:
for asset_name in ground_truth_state[asset_type].keys():
for state_name in ground_truth_state[asset_type][asset_name].keys():
runtime_asset_state = ground_truth_state[asset_type][asset_name][state_name][ground_truth_env_id]
dataset_asset_state = compared_state[asset_type][asset_name][state_name][0]
if len(dataset_asset_state) != len(runtime_asset_state):
return False, f"State shape of {state_name} for asset {asset_name} don't match"
for i in range(len(dataset_asset_state)):
if abs(dataset_asset_state[i] - runtime_asset_state[i]) > 0.01:
return (
False,
f'State ["{asset_type}"]["{asset_name}"]["{state_name}"][{i}] don\'t match\r\n',
)
return True, ""
def check_initial_state_recorder_term(env):
"""Check values recorded by the initial state recorder terms.
Args:
env: Environment instance.
"""
current_state = env.unwrapped.scene.get_state(is_relative=True)
for env_id in range(env.unwrapped.num_envs):
recorded_initial_state = env.unwrapped.recorder_manager.get_episode(env_id).get_initial_state()
are_states_equal, output_log = compare_states(recorded_initial_state, current_state, env_id)
assert are_states_equal, output_log
@pytest.mark.parametrize("task_name", ["Isaac-Lift-Cube-Franka-v0"])
@pytest.mark.parametrize("device", ["cuda:0", "cpu"])
@pytest.mark.parametrize("num_envs", [1, 2])
def test_action_state_recorder_terms(task_name, device, num_envs, temp_dir):
"""Check action state recorder terms."""
omni.usd.get_context().new_stage()
dummy_dataset_filename = f"{uuid.uuid4()}.hdf5"
# parse configuration
env_cfg = parse_env_cfg(task_name, device=device, num_envs=num_envs)
# set recorder configurations for this test
env_cfg.recorders = ActionStateRecorderManagerCfg()
env_cfg.recorders.dataset_export_dir_path = temp_dir
env_cfg.recorders.dataset_filename = dummy_dataset_filename
# create environment
env = gym.make(task_name, cfg=env_cfg)
# reset all environment instances to trigger post-reset recorder callbacks
env.reset()
check_initial_state_recorder_term(env)
# reset only one environment that is not the first one
env.unwrapped.reset(env_ids=torch.tensor([num_envs - 1], device=env.unwrapped.device))
check_initial_state_recorder_term(env)
# close the environment
env.close()
......@@ -11,7 +11,7 @@ from __future__ import annotations
"""Launch Isaac Sim Simulator first."""
from isaaclab.app import AppLauncher, run_tests
from isaaclab.app import AppLauncher
# launch omniverse app
app_launcher = AppLauncher(headless=True, enable_cameras=True)
......@@ -355,7 +355,3 @@ class TestScaleRandomization(unittest.TestCase):
with self.assertRaises(RuntimeError):
env = ManagerBasedEnv(cfg_failure)
env.close()
if __name__ == "__main__":
run_tests()
......@@ -9,7 +9,7 @@ This script tests the functionality of texture randomization applied to the cart
"""Launch Isaac Sim Simulator first."""
from isaaclab.app import AppLauncher, run_tests
from isaaclab.app import AppLauncher
# launch omniverse app
app_launcher = AppLauncher(headless=True, enable_cameras=True)
......@@ -199,8 +199,3 @@ class TestTextureRandomization(unittest.TestCase):
with self.assertRaises(RuntimeError):
env = ManagerBasedEnv(cfg_failure)
env.close()
if __name__ == "__main__":
# run the main function
run_tests()
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment