feat: Receive cycleNumber for Python bindings in output/execute/collect calls

[av-novikov]

cycleNumber is required for execute/output/collect calls in PyHistoryCollection.cpp, PyHistoryOutput.cpp, PyVTKOutput.cpp and PySolver.cpp. Currently, it is not passed from Python, but awkwardly calculated as int cycleNumber = int(round( time/dt )); which result in, for example, wrong indexation of VTK snapshots in case of dynamically changing timestep.

This PR support receiving of cycleNumber from Python in affected C++ calls.
Goes with geosPythonPackages: PR89.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.66%. Comparing base (078f90d) to head (34d9128).
Report is 19 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3631   +/-   ##
========================================
  Coverage    56.66%   56.66%           
========================================
  Files         1217     1217           
  Lines       105126   105126           
========================================
  Hits         59567    59567           
  Misses       45559    45559           

☔ 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.

[Victor-M-Gomes]

Hello Alexander, is it possible to have a check for cycleNumber? To make sure it has been parsed? That would avoid obliging us to modify all previous python codes.

Thanks.

static PyObject * collect( PyHistoryCollection * self, PyObject * args )
{
  VERIFY_NON_NULL_SELF( self );
  VERIFY_INITIALIZED( self );

  double time;
  double dt;
  int cycleNumber;
  int parsed_cycleNumber = 0; // Flag to track if cycleNumber was parsed

  if( !PyArg_ParseTuple( args, "ddi", &time, &dt, &cycleNumber ) )
  {
    // If parsing fails, it might be due to 'cycleNumber' not being provided.
    // Attempt to parse with only 'time' and 'dt'.
    if( !PyArg_ParseTuple( args, "dd", &time, &dt ) )
    {
      // If that also fails, then the initial arguments were incorrect.
      return nullptr;
    }
    // If parsing with "dd" succeeded, then 'cycleNumber' was not provided.
    cycleNumber = static_cast<int>(round( time / dt ));
    parsed_cycleNumber = 0; // Indicate that cycleNumber was calculated
  } else {
    parsed_cycleNumber = 1; // Indicate that cycleNumber was successfully parsed
  }

  geos::DomainPartition & domain = self->group->getGroupByPath< DomainPartition >( "/Problem/domain" );

  try
  {
    self->group->execute( time, dt, cycleNumber, 0, 0, domain );

[av-novikov]

Hello Alexander, is it possible to have a check for cycleNumber? To make sure it has been parsed? That would avoid obliging us to modify all previous python codes.

Thanks.

static PyObject * collect( PyHistoryCollection * self, PyObject * args )
{
  VERIFY_NON_NULL_SELF( self );
  VERIFY_INITIALIZED( self );

  double time;
  double dt;
  int cycleNumber;
  int parsed_cycleNumber = 0; // Flag to track if cycleNumber was parsed

  if( !PyArg_ParseTuple( args, "ddi", &time, &dt, &cycleNumber ) )
  {
    // If parsing fails, it might be due to 'cycleNumber' not being provided.
    // Attempt to parse with only 'time' and 'dt'.
    if( !PyArg_ParseTuple( args, "dd", &time, &dt ) )
    {
      // If that also fails, then the initial arguments were incorrect.
      return nullptr;
    }
    // If parsing with "dd" succeeded, then 'cycleNumber' was not provided.
    cycleNumber = static_cast<int>(round( time / dt ));
    parsed_cycleNumber = 0; // Indicate that cycleNumber was calculated
  } else {
    parsed_cycleNumber = 1; // Indicate that cycleNumber was successfully parsed
  }

  geos::DomainPartition & domain = self->group->getGroupByPath< DomainPartition >( "/Problem/domain" );

  try
  {
    self->group->execute( time, dt, cycleNumber, 0, 0, domain );

Dear @Victor-M-Gomes, thanks for checking!

1. Do I understand you correctly that you suggest to keep option to provide 2 arguments (time, dt) for the support of legacy code outside both repos?
2. Do you understand that it leaves a loophole for mistakes at least in output every time you change timestep?
3. Are you talking only about PyHistoryCollection or all affected classes? My concern is that wrong indexation in execute call may lead to mistakes beyond output.

I do not mind if you insist, but it is not a big change to support and cycleNumber = static_cast<int>(round( time / dt )) is ridiculous to me.

[Victor-M-Gomes]

Hello Alexander, is it possible to have a check for cycleNumber? To make sure it has been parsed? That would avoid obliging us to modify all previous python codes.
Thanks.

static PyObject * collect( PyHistoryCollection * self, PyObject * args )
{
  VERIFY_NON_NULL_SELF( self );
  VERIFY_INITIALIZED( self );

  double time;
  double dt;
  int cycleNumber;
  int parsed_cycleNumber = 0; // Flag to track if cycleNumber was parsed

  if( !PyArg_ParseTuple( args, "ddi", &time, &dt, &cycleNumber ) )
  {
    // If parsing fails, it might be due to 'cycleNumber' not being provided.
    // Attempt to parse with only 'time' and 'dt'.
    if( !PyArg_ParseTuple( args, "dd", &time, &dt ) )
    {
      // If that also fails, then the initial arguments were incorrect.
      return nullptr;
    }
    // If parsing with "dd" succeeded, then 'cycleNumber' was not provided.
    cycleNumber = static_cast<int>(round( time / dt ));
    parsed_cycleNumber = 0; // Indicate that cycleNumber was calculated
  } else {
    parsed_cycleNumber = 1; // Indicate that cycleNumber was successfully parsed
  }

  geos::DomainPartition & domain = self->group->getGroupByPath< DomainPartition >( "/Problem/domain" );

  try
  {
    self->group->execute( time, dt, cycleNumber, 0, 0, domain );

Dear @Victor-M-Gomes, thanks for checking!

1. Do I understand you correctly that you suggest to keep option to provide 2 arguments (time, dt) for the support of legacy code outside both repos?

2. Do you understand that it leaves a loophole for mistakes at least in output every time you change timestep?

3. Are you talking only about PyHistoryCollection or all affected classes? My concern is that wrong indexation in execute call may lead to mistakes beyond output.

I do not mind if you insist, but it is not a big change to support and cycleNumber = static_cast<int>(round( time / dt )) is ridiculous to me.

I see. Nevermind then. When the time comes we will make the necessary changes on our side.