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

[Model Update]: BatteryPass #40

Closed
5 of 6 tasks
jonbckr opened this issue Jan 23, 2023 · 7 comments
Closed
5 of 6 tasks

[Model Update]: BatteryPass #40

jonbckr opened this issue Jan 23, 2023 · 7 comments
Labels
MS1_Approved Checklist "MS1 Request for Model Developement" is approved.

Comments

@jonbckr
Copy link
Contributor

jonbckr commented Jan 23, 2023

Update Reason

  • FIELD batterie identification: description unclear what is it, potential vlues, missing examples
  • Cell chemistry – same as identification - unclear
  • Document: Field name is singular, description talks about set of documents – unclear
  • Manufacturer: Field name is manufacturer, description is LE that sells and invoices the battery. Seems wird. Either description should be entity who manufacturs the batterie or field name should be renamed
  • Address: Reference to BPDM ery generic, difficult to understand for consumer of the document. Where does he get further information?
  • Unclear on thoroughfare, locality postal delivery point, and premise attribute  isn’t that in conflict with BPDM BPN concept (site is a BPN)?
  • Contact entity: Shouldn’t this be also aligned with BPDM? Website, email, phone number. Are these very specific to the batterie, or related to the manufacturer?
  • Battery model: description says battery type  Typo/Copy paste
  • Manufacturer: Is there any reference to a globally valid manufacturer ID like the VDMA Manufacturer ID code, GS1 Global Location Number, Catena-X BPN, ….) Just haing a full text name does not seem to be sufficient
  • Entity name “material number and weight an percentage” is weird; shouldn’t there be a better common name ?
  • In physical dimension entity there is also weight included??? Unclear?

MS1 Criteria

  • The model that should be updated exists
  • The referenced use case exists
  • The potential updates are discussed with all stakeholders
  • The potential update will be either
    • backward compatible or
    • a (chain of) migration strategy(-ies) from all non-deprecated previous versions to the new model will be developed
@mthiers
Copy link
Contributor

mthiers commented Jan 23, 2023

core team round:
according to Kevin tram all bullet points can be ignored except for "Entity name “material number and weight an percentage” is weird; shouldn’t there be a better common name ?"

@mthiers
Copy link
Contributor

mthiers commented Jan 23, 2023

core team meeting:
"Entity name “material number and weight an percentage” is weird; shouldn’t there be a better common name ?" could not be found in neither version 2.0.0 nor 3.0.0

@tram Kevin will deliver feedback until EOB 23.02.2023

preliminary MS1 approval given
(@bs-jokri will do postdocumentation)

@bs-jokri bs-jokri added the MS1_Approved Checklist "MS1 Request for Model Developement" is approved. label Jan 23, 2023
@Kevinataccenture
Copy link

Kevinataccenture commented Jan 25, 2023

After refinement with Thorsten, the following points are added:

  • Change the the copyright from "Badische Anilin und Sodafabrik societates Europaea" to "BASF SE"
  • :BatteryVoltageCharacteristiC - large "C" at the end
  • :RatedCapacityCharacteristic - the description states "The total number of ampere-hours (Ah) ..." - the  bamm-c:unit unit:ampere. is ampere though, not ampere hours.
  • :OhmCharacterisitic - Typo. Characterisitic to be changed to Characteristic. Also in the bamm:preferredName "ohm characterisitic"@en;
  • Same type in :BatteryEnergyCharacterisitc, (and please again check the bamm:preferredName - same!)
  • Same typo in :CycleLifeTestCRateCharacterisitc
  • :capacityThresholdExhaustion - the bamm:preferredName here is wrong (looks like a copy-paste issue from the lines above). It's not "internal resistance".
  • :energyRoundtripEfficiencyChange - same as above, the preferredName should be "energy roundtrip efficiency change"@en;
  • :originalPower }}and {{originalPowerCapability look like they do the same job? Both have the same preferredName, too.
    The picture attached still refers to KilowattCharacteristic and WattCharacteristic - has this changed in v.3.? (TD: I cannot find it any more)
  • :batteryType and :batteryModel have the same description - copy-paste issue (see CMP-431 for a suggestion on the description)
  • Misspelled "material" in model with "matierial": :matierialPercentageMassFraction, :matierialWeight, :MatierialPercentageMassFractionCharacteristic 
  • Inconsistency in using a small letter for a property and a capital letter as first letter. Any reason for this?
    EnergyRoundtripEfficiencyChangeCharacterisitic   (looks like "PascalCase") but originalPower ("camelCase")
  • Add examples for Battery Identification: X123456789012X12345678901234567
  • Add examples for cellChemistry: LFP, Natrium, Li-Ion, NMC, NCA

@mthiers
Copy link
Contributor

mthiers commented Jan 25, 2023

Change the the copyright from "Badische Anilin und Sodafabrik societates Europaea" to "BASF SE" - DONE
:BatteryVoltageCharacteristiC - large "C" at the end - DONE changed to lower case
:RatedCapacityCharacteristic - the description states "The total number of ampere-hours (Ah) ..." - the bamm-c:unit unit:ampere. is ampere though, not ampere hours. - @Kevinataccenture : sooo.....???!?
:OhmCharacterisitic - Typo. Characterisitic to be changed to Characteristic. Also in the bamm:preferredName "ohm characterisitic"@en; - DONE changed to all occurences of "Characterisitic" to "Characteristic"
Same type in :BatteryEnergyCharacterisitc, (and please again check the bamm:preferredName - same!)
Same typo in :CycleLifeTestCRateCharacterisitc
:capacityThresholdExhaustion - the bamm:preferredName here is wrong (looks like a copy-paste issue from the lines above). It's not "internal resistance". - DONE change to "capacity threshold exhaustion"
:energyRoundtripEfficiencyChange - same as above, the preferredName should be "energy roundtrip efficiency change"@en; - DONE changed to "energy roundtrip efficiency change"
:originalPower }}and {{originalPowerCapability look like they do the same job? Both have the same preferredName, too.
The picture attached still refers to KilowattCharacteristic and WattCharacteristic - has this changed in v.3.? (TD: I cannot find it any more) - @Kevinataccenture : sooo.....???!?
:batteryType and :batteryModel have the same description - copy-paste issue (see CMP-431 for a suggestion on the description) - @Kevinataccenture : sooo.....???!? the example does not help
Misspelled "material" in model with "matierial": :matierialPercentageMassFraction, :matierialWeight, - DONE changed all occurences of "matierial" to "material"
:MatierialPercentageMassFractionCharacteristic 
Inconsistency in using a small letter for a property and a capital letter as first letter. Any reason for this?
EnergyRoundtripEfficiencyChangeCharacterisitic (looks like "PascalCase") but originalPower ("camelCase") - yes, conventions
Add examples for Battery Identification: X123456789012X12345678901234567 - wrong place for example, already included in model see "batteryIDDMCCode"
Add examples for cellChemistry: LFP, Natrium, Li-Ion, NMC, NCA - wrong place for example, already included in model see "battery type"

@mthiers
Copy link
Contributor

mthiers commented Jan 25, 2023

@DanielSteffenReichert

after clarification of left over comments model needs debugging:

BatteryPass.zip

@Kevinataccenture
Copy link

:RatedCapacityCharacteristic - the description states "The total number of ampere-hours (Ah) ..." - the bamm-c:unit unit:ampere. is ampere though, not ampere hours. - @Kevinataccenture : sooo.....???!?
--> change line 537 in TTL to ampere-hours

:originalPower }}and {{originalPowerCapability look like they do the same job? Both have the same preferredName, too.
The picture attached still refers to KilowattCharacteristic and WattCharacteristic - has this changed in v.3.? (TD: I cannot find it any more) - @Kevinataccenture : sooo.....???!?
--> need to be clarified

:batteryType and :batteryModel have the same description - copy-paste issue (see CMP-431 for a suggestion on the description) - @Kevinataccenture : sooo.....???!? the example does not help
--> change description of BatteryModel: change battery type to batterymodel

@bs-jokri
Copy link
Contributor

bs-jokri commented Mar 6, 2023

closed with #68

@bs-jokri bs-jokri closed this as completed Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MS1_Approved Checklist "MS1 Request for Model Developement" is approved.
Projects
None yet
Development

No branches or pull requests

4 participants