Conversation
Values which are not numbers separated by ":" or "," are now used as they are, so NCrystal cfg-strings like "Ge_sg227.ncmat;dir1=@crys_hkl:5,1,1@lab:0,0,1" (or Windows paths like "C:\data\a.dat") can be given on the command line. With -L, values are still lists separated by commas, where commas within an entry can now be escaped as "\,". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Cool @tkittel, indeed great (but also busy) times with these new code-slaves. 🤖 🤣 I have a slurm batch somewhere that does a multi-dimensional scan with NCrystal strings, let me run that in addition to the PR tests - will merge if that also comes back healthy. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After iterations with @willend, me and our coding slave Claude, this now fixes yet another place where one could not easily pass through NCrystal cfg-strings.
I did review the changes - and claude wrote the rest here
mcrun took every parameter value containing
:or,as scan syntax. So NCrystal cfg-strings (and e.g. Windows paths) with such characters could not be given on the command line:Values with
::(stdlib::...) skipped the colon check. But they then went to the comma parsing, where a value with commas was dropped (and the instrument ran with the default).Now only values made of numbers separated by
:or,are scan syntax. Other values are used as they are. This replaces the special case for::. With-L, values are still lists separated by commas, as before. A comma within a list entry can now be escaped as\,, which is mentioned in the-Lhelp:Tested by comparing with the current mcrun on 44 command lines:
-Nranges (also negative and exponent numbers),a:delta:b,-M(also with-N2,3and mixed syntaxes),-Lwith lists of file names and cfg-strings (also with::),-Lwithmin:delta:max,--optimize,--seeds;x=1:2,x=1,2,,x=.dir1=@crys_hkl:5,1,1...;C:\data\a.dat, also in-Llists;-Lentries with\,;f=a.dat,b.datwithout-L, which is now that literal string instead of being dropped.x=1:a:3is now rejected by the instrument ("Invalid value '1:a:3' for floating point parameter x") instead of by mcrun;x=1::3is the other way round;-N2 f=b.dat,c.datnow gives "No interval range specified" instead of a Python exception.f=a.dat,) is still ignored with a warning, as before.🤖 Generated with Claude Code