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

#133 CriticalTime Calculation implementation done. Tests added in API… #136

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AlexPoiron
Copy link

closes #133

nrt/data/IEEE14/CTC/reference/aggregatedResults.xml Outdated Show resolved Hide resolved
sources/API/aggregatedResults/DYNAggrResXmlExporter.cpp Outdated Show resolved Hide resolved
sources/API/aggregatedResults/DYNAggrResXmlExporter.cpp Outdated Show resolved Hide resolved
sources/API/aggregatedResults/DYNAggrResXmlExporter.h Outdated Show resolved Hide resolved
sources/API/multipleJobs/DYNMultipleJobsXmlHandler.h Outdated Show resolved Hide resolved
sources/Launcher/DYNCriticalTimeLauncher.cpp Outdated Show resolved Hide resolved
sources/main.cpp Outdated Show resolved Hide resolved
sources/main.cpp Outdated Show resolved Hide resolved
sources/main.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@rosiereflo rosiereflo left a comment

Choose a reason for hiding this comment

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

Could you also please add a IEEE14 based nrt?

@@ -101,6 +101,34 @@ XmlExporter::exportLoadIncreaseResultsToStream(const vector<LoadIncreaseResult>&
formatter->endDocument();
}

void
XmlExporter::exportCriticalTimeResultsToFile(double criticalTime, const std::string& messageCriticalTimeError, std::string filePath) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

const std::string& filePath

/**
* @brief Export critical time calculation results into a file
*
* @param results aggregated results to export
Copy link
Contributor

Choose a reason for hiding this comment

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

doxygen is not correct (same below)


void
CriticalTimeCalculation::setAccuracy(double accuracy) {
if (accuracy < 0 || accuracy > 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

if (accuracy < 0 || accuracy > 1 || doubleIsZero(accuracy))

}

void
CriticalTimeCalculation::setJobsFile(std::string jobsFile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

const std::string&

dydId_ = dydId;
}

std::string
Copy link
Contributor

Choose a reason for hiding this comment

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

const std::string&

// First simulation case.
SetParametersAndLaunchSimulation();
if (results_.getSuccess())
tSup_ += std::round(((tSup_ - tInf) / 2) * multiplierRound) / multiplierRound;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any check that tInf < tSup

}

void
CriticalTimeLauncher::updateIndexes(double& tPrevious, double& curAccuracy, const double& multiplierRound) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this method is small and called only once in the code I would propose to remove it and put the code directly below

if (!results_.getSuccess())
tSup_ -= accuracy;

tSup_ = std::round(tSup_ * multiplierRound) / multiplierRound;;
Copy link
Contributor

Choose a reason for hiding this comment

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

additional ;

//

/**
* @file DYNMarginCriticalimeLauncher.h
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: CriticalTime


std::string simulationMC_SA_CS_CTC = "Set the simulation type to launch : MC (Margin calculation), SA (systematic analysis), CS (compute simulation)"
" or CTC (critical time calculation)";
const char* allSimulationsType = simulationMC_SA_CS_CTC.c_str();
Copy link
Contributor

Choose a reason for hiding this comment

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

useless, please use directly simulationMC_SA_CS_CTC.c_str() below

@rosiereflo rosiereflo marked this pull request as draft March 8, 2022 09:41
AlexPoiron and others added 3 commits November 4, 2022 10:43
@gautierbureau
Copy link
Member

I updated with a recent master and added some fix to make the branch work. To make the test case work with Dynawo temporary we need to add the following lines in dynawo/sources/Models/Modelica/Dynawo/Electrical/Machines/OmegaRef/BaseClasses.mo

import Modelica.Constants;
assert(not (theta > Constants.pi and time > 0.1), "theta > PI");

@FredericSabot
Copy link
Contributor

From my understanding, only the first job of the fic_multiple is read. But it could be useful to compute the CCTs of all generators in one go (possibly using the "dydFile" entry).

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.

Critical time calculation (CTC) Implementation
4 participants