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.

Sign up for GitHub

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

Jump to bottom

Feature/107 fix bugs security issues and maintenance issues in mccl and mcpp #108

Merged
merged 3 commits into from
Oct 1, 2023
Merged

Feature/107 fix bugs security issues and maintenance issues in mccl and mcpp #108

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e5a24c
Fix the SonarCloud issues in MCCL and MCPP
Sep 30, 2023
a0e2d84
Removed the CommandLineTests.cs file from MCPP because it is identica…
Sep 30, 2023
678f72c
Added the specific version to the header
Oct 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,4 @@ src/README.md
src/Vts.MonteCarlo.CommandLineApplication/Properties/launchSettings.json
.sonarqube/
src/Vts/Vts.xml
src/.github/
8 changes: 4 additions & 4 deletions src/Vts.MonteCarlo.CommandLineApplication/CommandLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ public Switch(string name, Action<IEnumerable<string>> handler)
Handler = handler;
}

public string Name { get; private set; }
public string ShortForm { get; private set; }
public Action<IEnumerable<string>> Handler { get; private set; }
public string Name { get; }
public string ShortForm { get; }
public Action<IEnumerable<string>> Handler { get; }

public int InvokeHandler(string[] values)
{
Expand All @@ -37,7 +37,7 @@ public int InvokeHandler(string[] values)
/* The regex that extracts names and comma-separated values for switches
in the form (<switch>[="value 1",value2,...])+ */
private static readonly Regex ArgRegex =
new Regex(@"(?<name>[^=]+)=?((?<quoted>\""?)(?<value>(?(quoted)[^\""]+|[^,]+))\""?,?)*",
new(@"(?<name>[^=]+)=?((?<quoted>\""?)(?<value>(?(quoted)[^\""]+|[^,]+))\""?,?)*",
RegexOptions.Compiled | RegexOptions.CultureInvariant |
RegexOptions.ExplicitCapture | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(10));

Expand Down
151 changes: 74 additions & 77 deletions src/Vts.MonteCarlo.CommandLineApplication/Program.cs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is suggested update to local variable CPUCount name cpuCount and not cPuCount? I found an example of local variable in pMCDatabase.cs of pMCDatabase and suggested name is pMcDatabase.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CPU is an acronym like API and are considered words like Count and Database, putting an uppercase P in the middle is not right, I guess it made that suggestion incorrectly, it should be pmcDatabase in camel case and PmcDatabase in Pascal case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks for your response.

Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//#define PROCESS_ATTACH_DEBUG

using NLog;
using System;
using System.Collections.Generic;
Expand All @@ -9,15 +7,10 @@
using Vts.Common.Logging;

[assembly: InternalsVisibleTo("Vts.MonteCarlo.CommandLineApplication.Test")]
[assembly: InternalsVisibleTo("Vts.MonteCarlo.PostProcessor.CommandLineApplication.Test")]

namespace Vts.MonteCarlo.CommandLineApplication
{

#region CommandLine Arguments Parser

/* Simple commandline argument parser written by Ananth B. http://www.ananthonline.net */

#endregion
/// <summary>
/// Monte Carlo command line application program. Type "mc help" for
/// a description of the different command line parameters.
Expand All @@ -35,15 +28,11 @@ public static class Program
/// <returns>int = 2 (infile exists but does not pass validation)</returns>
public static int Main(string[] args)
{
#if PROCESS_ATTACH_DEBUG
Console.Read();
#endif
var inFile = "";
var inFiles = new List<string>();
var outName = "";
var outPath = "";
var CPUCount = "1"; // default is to use 1
var infoOnlyOption = false;
var cpuCount = "1"; // default is to use 1
IList<ParameterSweep> paramSweep = new List<ParameterSweep>();

args.Process(() =>
Expand All @@ -59,12 +48,10 @@ public static int Main(string[] args)
ShowHelp(helpTopic);
else
ShowHelp();
infoOnlyOption = true;
}),
new CommandLine.Switch("geninfiles", val =>
{
GenerateDefaultInputFiles();
infoOnlyOption = true;
}),
new CommandLine.Switch("infile", val =>
{
Expand All @@ -91,21 +78,21 @@ public static int Main(string[] args)
}),
new CommandLine.Switch("cpucount", val =>
{
CPUCount = val.First();
if (CPUCount == "all")
cpuCount = val.First();
if (cpuCount == "all")
{
CPUCount = Environment.ProcessorCount.ToString();
Logger.Info(() => "changed to maximum CPUs on system " + CPUCount);
cpuCount = Environment.ProcessorCount.ToString();
Logger.Info(() => "changed to maximum CPUs on system " + cpuCount);
}
else
{
if (!int.TryParse(CPUCount, out var CPUCountInt))
if (!int.TryParse(cpuCount, out var cpuCountInt))
{
Logger.Info(() => "unknown cpucount option " + CPUCount);
Logger.Info(() => $"unknown cpucount option {cpuCount}");
}
else
{
Logger.Info(() => "number of CPUs specified as " + CPUCount);
Logger.Info(() => $"number of CPUs specified as {cpuCountInt}");
}
}
}),
Expand Down Expand Up @@ -134,9 +121,13 @@ public static int Main(string[] args)
Logger.Info(() => "parameter sweep specified as " + sweepString[0] + " values");
}));

if (!infoOnlyOption)
if (CheckInfoOnly(args))
{
Func<SimulationInput, bool> checkValid = simInput =>
LogManager.Configuration = null;
return 0;
}

Func<SimulationInput, bool> checkValid = simInput =>
{
var validationResult = MonteCarloSetup.ValidateSimulationInput(simInput);
if (validationResult.IsValid) return true;
Expand All @@ -145,80 +136,86 @@ public static int Main(string[] args)
Console.Write("\nRemarks:" + validationResult.Remarks);
return false;
};
SimulationInput input;
if (paramSweep.Any() || inFiles.Any())
SimulationInput input;
if (paramSweep.Any() || inFiles.Any())
{
IList<SimulationInput> inputs;
if (paramSweep.Any())
{
IList<SimulationInput> inputs;
if (paramSweep.Any())
{
input = MonteCarloSetup.ReadSimulationInputFromFile(inFile);
if (input == null)
{
return 1;
}
if (!string.IsNullOrEmpty(outName))
{
input.OutputName = outName;
}

inputs = MonteCarloSetup.ApplyParameterSweeps(input, paramSweep).ToList();
}
else // if infiles.Count() > 0
input = MonteCarloSetup.ReadSimulationInputFromFile(inFile);
if (input == null)
{
inputs = inFiles.Select(file => MonteCarloSetup.ReadSimulationInputFromFile(file)).ToList();
if (!inputs.Any())
{
return 1;
}
return 1;
}
// validate input
if (inputs.Any(simulationInput => !checkValid(simulationInput)))
if (!string.IsNullOrEmpty(outName))
{
return 2;
}
// make sure input does not specify Database if CPUCount>1
if (int.Parse(CPUCount) > 1 && (inputs.First().Options.Databases != null && inputs.First().Options.Databases.Count != 0))
{
CPUCount = 1.ToString();
Logger.Info(() => "parallel processing cannot be performed when a Database is specified, changed CPUCount to 1");
input.OutputName = outName;
}

MonteCarloSetup.RunSimulations(inputs, outPath, int.Parse(CPUCount));
Logger.Info("\nSimulations complete.");
return 0;
inputs = MonteCarloSetup.ApplyParameterSweeps(input, paramSweep).ToList();
}

input = MonteCarloSetup.ReadSimulationInputFromFile(inFile);
if (input == null)
else // if infiles.Count() > 0
{
return 1;
inputs = inFiles.Select(file => MonteCarloSetup.ReadSimulationInputFromFile(file)).ToList();
if (!inputs.Any())
{
return 1;
}
}

if (!checkValid(input))
return 2;

if (!string.IsNullOrEmpty(outName))
// validate input
if (inputs.Any(simulationInput => !checkValid(simulationInput)))
{
input.OutputName = outName;
return 2;
}

// make sure input does not specify Database if CPUCount>1
if (int.Parse(CPUCount) > 1 && (input.Options.Databases != null && input.Options.Databases?.Count != 0))
if (int.Parse(cpuCount) > 1 && (inputs.First().Options.Databases != null && inputs.First().Options.Databases.Count != 0))
{
CPUCount = 1.ToString();
Logger.Info(() =>
"parallel processing cannot be performed when a Database is specified, changed CPUCount to 1");
cpuCount = 1.ToString();
Logger.Info(() => "parallel processing cannot be performed when a Database is specified, changed CPUCount to 1");
}

MonteCarloSetup.RunSimulation(input, outPath, int.Parse(CPUCount));
Logger.Info("\nSimulation complete.");
MonteCarloSetup.RunSimulations(inputs, outPath, int.Parse(cpuCount));
Logger.Info("\nSimulations complete.");
return 0;
}

LogManager.Configuration = null;
input = MonteCarloSetup.ReadSimulationInputFromFile(inFile);
if (input == null)
{
return 1;
}

if (!checkValid(input))
return 2;

if (!string.IsNullOrEmpty(outName))
{
input.OutputName = outName;
}

// make sure input does not specify Database if CPUCount>1
if (int.Parse(cpuCount) > 1 && (input.Options.Databases != null && input.Options.Databases?.Count != 0))
{
cpuCount = 1.ToString();
Logger.Info(() =>
"parallel processing cannot be performed when a Database is specified, changed CPUCount to 1");
}

MonteCarloSetup.RunSimulation(input, outPath, int.Parse(cpuCount));
Logger.Info("\nSimulation complete.");
return 0;
}

private static bool CheckInfoOnly(IEnumerable<string> args)
{
foreach (var arg in args)
{
if (arg.StartsWith("help")) return true;
if (arg.Equals("geninfiles")) return true;
}
return false;
}

private static void GenerateDefaultInputFiles()
{
try
Expand Down
34 changes: 0 additions & 34 deletions src/Vts.MonteCarlo.PostProcessor.Test/CommandLineTests.cs
View file
Open in desktop

This file was deleted.

19 changes: 6 additions & 13 deletions src/Vts.MonteCarlo.PostProcessor/Program.cs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@

namespace Vts.MonteCarlo.PostProcessor
{
#region CommandLine Arguments Parser

/* Simple commandline argument parser written by Ananth B. http://www.ananthonline.net */

#endregion

public static class Program
{
/// <summary>
Expand All @@ -23,14 +17,10 @@ public static class Program
/// <returns>int = 2 (infile exists but does not pass validation)</returns>
public static int Main(string[] args)
{
#if PROCESS_ATTACH_DEBUG
Console.Read();
#endif
var inFile = "infile.txt";
var inPath = "";
var outName = "";
var outPath = "";
var infoOnlyOption = false;
args.Process(() =>
{
Console.WriteLine(@"Virtual Photonics MC Post-Processor 1.0");
Expand All @@ -40,13 +30,11 @@ public static int Main(string[] args)
},
new CommandLine.Switch("help", val =>
{
infoOnlyOption = true;
ShowHelp();
}),
new CommandLine.Switch("geninfiles", val =>
{
GenerateDefaultInputFiles();
infoOnlyOption = true;
}),
new CommandLine.Switch("infile", val =>
{
Expand All @@ -70,7 +58,7 @@ public static int Main(string[] args)
})
);

if (infoOnlyOption) return 0;
if (CheckInfoOnly(args)) return 0;
var input = PostProcessorSetup.ReadPostProcessorInputFromFile(inFile);
if (input == null)
{
Expand All @@ -94,6 +82,11 @@ public static int Main(string[] args)
return 0;
}

private static bool CheckInfoOnly(string[] args)
{
return args.Contains("help") || args.Contains("geninfiles");
}

private static void GenerateDefaultInputFiles()
{
var infiles = PostProcessorInputProvider.GenerateAllPostProcessorInputs();
Expand Down
Loading