Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing overlaping sections bug #4915

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aleynaakyuz
Copy link
Contributor

This PR adds a check for repeated options in configuration files used for inference jobs. Previously, if an option was repeated across multiple configuration files, the latest instance would overwrite earlier ones without raising an error. Now, the code detects repeated options and raises an error.

@ahnitz
Copy link
Member

ahnitz commented Oct 28, 2024

@aleynaakyuz Can you check the failed template generation output to see if you change is causing this? This might be something we want to change in the example if so. Otherwise, it may be an unrelated problem, but we should determine that before accepting this change.

@ahnitz ahnitz requested a review from spxiwh October 28, 2024 20:42
@spxiwh
Copy link
Contributor

spxiwh commented Oct 29, 2024

Thanks for proposing this fix! I was surprised that the ConfigParser library doesn't let us do this (still). However, I couldn't see any way that it would do it, so I think this is a good solution.

I'd be happy to approve once this passes the test suite.

@aleynaakyuz
Copy link
Contributor Author

@ahnitz The error raised by the test is the following

        Traceback (most recent call last):
          File "/home/aakyuz/miniconda3/envs/workflow_python/bin/pycbc_geom_aligned_bank", line 561, in <module>
            workflow = wf.Workflow(opts)
                       ^^^^^^^^^^^^^^^^^
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/site-packages/pycbc/workflow/core.py", line 673, in __init__
            self.cp = WorkflowConfigParser.from_cli(args)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/site-packages/pycbc/types/config.py", line 227, in from_cli
            return cls(opts.config_files, overrides, deleteTuples=deletes)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/site-packages/pycbc/workflow/configuration.py", line 328, in __init__
            InterpolatingConfigParser.__init__(
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/site-packages/pycbc/types/config.py", line 120, in __init__
            self.read_ini_file(configFiles)
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/site-packages/pycbc/types/config.py", line 252, in read_ini_file
            parser.read(filename)
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/configparser.py", line 712, in read
            self._read(fp, filename)
          File "/home/aakyuz/miniconda3/envs/workflow_python/lib/python3.11/configparser.py", line 1111, in _read
            raise DuplicateOptionError(sectname, optname,
        configparser.DuplicateOptionError: While reading from 'temp.ini' [line 28]: option '_condor_schedd_daemon_ad_file' in section 'environment' already exists

The option is repeated in the same section but as the following
_CONDOR_SCHEDD_DAEMON_AD_FILE = /var/spool/condor/.schedd_classad
_condor_SCHEDD_DAEMON_AD_FILE = /var/spool/condor/.schedd_classad

As far as I understand, configparser is not case-sensitive (it raises duplicate options error regardless of upper case or lower case). However, pycbc code enables the case sensitivity here so reading the files with self.read doesn't raise the same error.

I couldn't understand why that option is repeated in a weird way so I don't know how to get rid of it yet.

@spxiwh
Copy link
Contributor

spxiwh commented Nov 28, 2024

I just wanted to check if this was waiting on me? I didn't look over it while the tests are failing, but can do if you want me to??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants